[go_router_builder] Removes `path_to_regexp` from the dependencies #4524

ValentinVignal · 2023-07-20T02:55:14Z

Fixes flutter/flutter#130817
Relates to flutter/flutter#122713

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2023-07-20T02:55:16Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

ValentinVignal · 2023-07-20T02:57:20Z

packages/go_router_builder/lib/src/path_utils.dart

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'route.dart';
+
+final RegExp _parameterRegExp = RegExp(r':(\w+)(\((?:\\.|[^\\()])+\))?');
+
+/// Converts a [pattern] such as `/user/:id` into [RegExp].
+///
+/// The path parameters can be specified by prefixing them with `:`. The
+/// `parameters` are used for storing path parameter names.
+///
+///
+/// For example:
+///
+///  `pattern` = `/user/:id/book/:bookId`
+///
+///  The `parameters` would contain `['id', 'bookId']` as a result of calling
+///  this method.
+///
+/// To extract the path parameter values from a [RegExpMatch], pass the
+/// [RegExpMatch] into [extractPathParameters] with the `parameters` that are
+/// used for generating the [RegExp].
+RegExp patternToRegExp(String pattern, List<String> parameters) {
+  final StringBuffer buffer = StringBuffer('^');


This file in this commit (chore: Copy path_utils.dart from go_router) is a literal copy past from go_router's path_utils.dart file https://github.com/flutter/packages/blob/main/packages/go_router/lib/src/path_utils.dart

ValentinVignal · 2023-07-20T02:57:57Z

packages/go_router_builder/lib/src/path_utils.dart

-
-/// Extracts arguments from the `match` and maps them by parameter name.
-///
-/// The [parameters] should originate from the call to [patternToRegExp] that
-/// creates the [RegExp].
-Map<String, String> extractPathParameters(
-    List<String> parameters, RegExpMatch match) {
-  return <String, String>{
-    for (int i = 0; i < parameters.length; ++i)
-      parameters[i]: match.namedGroup(parameters[i])!
-  };
-}
-
-/// Concatenates two paths.
-///


In this commit (revamp: Remove unused methods) I'm only deleting methods that are not used

ValentinVignal · 2023-07-20T02:59:52Z

packages/go_router_builder/lib/src/path_utils.dart

-///  `pattern` = `/user/:id/book/:bookId`
-///
-///  The `parameters` would contain `['id', 'bookId']` as a result of calling
-///  this method.
-///
-/// To extract the path parameter values from a [RegExpMatch], pass the
-/// [RegExpMatch] into [extractPathParameters] with the `parameters` that are
-/// used for generating the [RegExp].
-RegExp patternToRegExp(String pattern, List<String> parameters) {
-  final StringBuffer buffer = StringBuffer('^');
-  int start = 0;
-  for (final RegExpMatch match in _parameterRegExp.allMatches(pattern)) {
-    if (match.start > start) {
-      buffer.write(RegExp.escape(pattern.substring(start, match.start)));
-    }
-    final String name = match[1]!;
-    final String? optionalPattern = match[2];
-    final String regex = optionalPattern != null
-        ? _escapeGroup(optionalPattern, name)
-        : '(?<$name>[^/]+)';
-    buffer.write(regex);
-    parameters.add(name);
-    start = match.end;
-  }
-
-  if (start < pattern.length) {
-    buffer.write(RegExp.escape(pattern.substring(start)));
-  }
-
-  if (!pattern.endsWith('/')) {
-    buffer.write(r'(?=/|$)');
-  }
-  return RegExp(buffer.toString(), caseSensitive: false);
-}


In this commit (refactor: Simplify path utils methods to only keep what is used),

I'm only deleting and simplifying patternToRegExp. We don't need to use the returned RegExp. So instead I'm returning the parameters (and I don't need the parameter parameters anymore)

ValentinVignal · 2023-07-20T03:03:25Z

I believe this PR can have a no-test exemption. This is a revamp and the current tests should cover it already

chunhtai

can you remove the path_to_regexp in script/configs/allowed_unpinned_deps.yaml, too?

chunhtai · 2023-07-20T15:54:58Z

packages/go_router_builder/pubspec.yaml

@@ -2,7 +2,7 @@ name: go_router_builder
 description: >-
  A builder that supports generated strongly-typed route helpers for
  package:go_router
-version: 2.2.2
+version: 2.3.0


There isn't a new feature, should probably update patch version.

Sure, I did it in chore: Only update patch version

ValentinVignal · 2023-07-20T16:29:24Z

can you remove the path_to_regexp in script/configs/allowed_unpinned_deps.yaml, too?

I removed it in ci: Remove path_to_regexp from allowed unpinned deps

chunhtai · 2023-07-20T16:35:50Z

It looks like the branch is outdated so the ci.yaml failed. can you rebase off latest main?

chunhtai

LGTM

…-dependencies-2

stuartmorgan-g

LGTM, thanks!

hannah-hyj

LGTM

Hixie · 2023-07-20T20:12:33Z

it's probably worth throwing in a few unit tests for the functions in packages/go_router_builder/lib/src/path_utils.dart, just so that the next person to refactor them can be confident they didn't break anything subtle about that code.

chunhtai

As @Hixie stated, please add some tests and re-request my review once done

…emove-path-to-regexp-from-dependencies-2

ValentinVignal · 2023-07-21T01:17:39Z

tests: Add path_utils unit test

I added some unit tests in tests: Add path_utils unit test

chunhtai

LGTM

…ncies (flutter/packages#4524)

flutter/packages@2266a76...8028caf 2023-07-24 [email protected] Roll Flutter from a8c8c55 to d7ed5dc (7 revisions) (flutter/packages#4556) 2023-07-23 [email protected] Roll Flutter from e8b397c to a8c8c55 (11 revisions) (flutter/packages#4552) 2023-07-22 [email protected] Roll Flutter from 9cfbf6b to e8b397c (12 revisions) (flutter/packages#4550) 2023-07-21 [email protected] [ci] Clean up except exclusion list (flutter/packages#4547) 2023-07-21 [email protected] [go_router] Replaces uri related properties in GoRouterState (flutter/packages#4392) 2023-07-21 [email protected] [webview_flutter_wkwebview] Updates pigeon version to `10.1.4` (flutter/packages#4548) 2023-07-21 [email protected] [pigeon] Adds package name to method channel. (flutter/packages#4443) 2023-07-21 [email protected] [ci] Migrate remaing unblocked repo checks to LUCI (flutter/packages#4543) 2023-07-21 [email protected] [go_router] Remove unused dependency (flutter/packages#4545) 2023-07-21 [email protected] Roll Flutter from d07e8ae to 9cfbf6b (58 revisions) (flutter/packages#4544) 2023-07-21 [email protected] Adding myself and Michele to AUTHORS (flutter/packages#4527) 2023-07-21 [email protected] [go_router_builder] Removes `path_to_regexp` from the dependencies (flutter/packages#4524) 2023-07-21 [email protected] [google_maps_flutter_web] Adds options for gesture handling and tilt controls. (flutter/packages#4521) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

ValentinVignal added 6 commits July 20, 2023 10:23

chore: Copy path_utils.dart from go_router

c9487b3

revamp: Remove unused methods

0cc50db

refactor: Simplify path utils methods to only keep what is used

205da04

chore: Remove path_to_regexp dependency

e48e2af

refactor: Update the code to use path_utils

31a3785

chore: Increase package version

f898d88

ValentinVignal requested a review from chunhtai as a code owner July 20, 2023 02:55

github-actions bot added the p: go_router_builder label Jul 20, 2023

ValentinVignal commented Jul 20, 2023

View reviewed changes

ValentinVignal mentioned this pull request Jul 20, 2023

[go_router_builder] Removes path_to_regexp from the dependencies #4517

Closed

11 tasks

chunhtai requested a review from hannah-hyj July 20, 2023 15:51

chunhtai reviewed Jul 20, 2023

View reviewed changes

ValentinVignal added 2 commits July 21, 2023 00:26

chore: Only update patch version

57f71a9

ci: Remove path_to_regexp from allowed unpinned deps

5969d6f

ValentinVignal requested a review from chunhtai July 20, 2023 16:29

chunhtai approved these changes Jul 20, 2023

View reviewed changes

chunhtai requested a review from stuartmorgan-g July 20, 2023 16:36

Merge branch 'main' into go-router-builder/remove-path-to-regexp-from…

b85acdb

…-dependencies-2

stuartmorgan-g approved these changes Jul 20, 2023

View reviewed changes

hannah-hyj approved these changes Jul 20, 2023

View reviewed changes

chunhtai requested changes Jul 20, 2023

View reviewed changes

ValentinVignal added 2 commits July 21, 2023 09:16

tests: Add path_utils unit test

d8391de

Merge remote-tracking branch 'upstream/main' into go-router-builder/r…

79e3bb6

…emove-path-to-regexp-from-dependencies-2

ValentinVignal requested a review from chunhtai July 21, 2023 01:17

chunhtai approved these changes Jul 21, 2023

View reviewed changes

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 21, 2023

auto-submit bot merged commit 1af4d1a into flutter:main Jul 21, 2023

engine-flutter-autoroll mentioned this pull request Jul 24, 2023

Roll Packages from 2266a7618ee3 to aad7b44b439b (12 revisions) flutter/flutter#131193

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 24, 2023

1af4d1a [go_router_builder] Removes path_to_regexp from the depende…

aa85693

…ncies (flutter/packages#4524)

engine-flutter-autoroll mentioned this pull request Jul 24, 2023

Roll Packages from 2266a7618ee3 to 8028cafa1f0c (13 revisions) flutter/flutter#131196

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 24, 2023

1af4d1a [go_router_builder] Removes path_to_regexp from the depende…

929f88f

…ncies (flutter/packages#4524)

reidbaker mentioned this pull request Jul 26, 2023

[Tooling] Add google owned cache for dependencies as an option in ci #4567

Merged

11 tasks

reidbaker mentioned this pull request Aug 4, 2023

[All] Expand artifact hub to all plugins #4645

Merged

11 tasks

[go_router_builder] Removes path_to_regexp from the dependencies #4524

[go_router_builder] Removes path_to_regexp from the dependencies #4524

Uh oh!

Conversation

ValentinVignal commented Jul 20, 2023

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jul 20, 2023

Uh oh!

ValentinVignal Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValentinVignal Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

ValentinVignal Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValentinVignal commented Jul 20, 2023

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

ValentinVignal Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

ValentinVignal commented Jul 20, 2023

Uh oh!

chunhtai commented Jul 20, 2023

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

hannah-hyj left a comment

Choose a reason for hiding this comment

Uh oh!

Hixie commented Jul 20, 2023

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

ValentinVignal commented Jul 21, 2023

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[go_router_builder] Removes `path_to_regexp` from the dependencies #4524

[go_router_builder] Removes `path_to_regexp` from the dependencies #4524

ValentinVignal Jul 20, 2023 •

edited

Loading

ValentinVignal Jul 20, 2023 •

edited

Loading