This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Fixes a few issues in flutter_js #53231
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This addresses a couple things: flutter/flutter#147610 (Treat `auto` renderer properly) flutter/flutter#149443 (Add an entrypoint base url config option) This also adds a `useLocalCanvasKit` configuration option on the build config, which the flutter tool can use to fix flutter/flutter#148713
Contributor
Author
|
I'm testing these changes against the framework presubmit stuff here: flutter/flutter#149769 |
Contributor
Author
|
Presubmit testing against the framework has some issues currently. (flutter/flutter#149780) But it looks the presubmit testing that I did get to run seems to indicate things are working with this change. |
ditman
approved these changes
Jun 7, 2024
Member
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
Thanks for the clean up! All I can do is nitpick, I think the worst offender is the introduction of some single quotes in the new file utils.js (while it seems we're going to double quotes everywhere elsewhere)
lib/web_ui/flutter_js/src/utils.js
Outdated
Comment on lines
26
to
32
| function stripLeftSlash(s) { | ||
| return s.startsWith('/') ? s.substring(1) : s; | ||
| } | ||
|
|
||
| function stripRightSlash(s) { | ||
| return s.endsWith('/') ? s.substring(0, s.length - 1) : s; | ||
| } |
Member
There was a problem hiding this comment.
Do we want these to strip repeated slashes?
Contributor
Author
There was a problem hiding this comment.
Hmmm, I thought about that. I was on the fence about it. Seems like the code would be more complex, and passing in a path segment with more than one leading or trailing slash feels like an obviously wrong thing to do and if the path segment ends up malformed then whatever. But then again, maybe it's just better to strip multiple slashes. I'll look to see if I can make a simple change here.
lib/web_ui/flutter_js/src/utils.js
Outdated
| } | ||
|
|
||
| export function joinPathSegments(...segments) { | ||
| return segments.map((segment, i) => { |
Member
There was a problem hiding this comment.
segments
.filter(s => s != null)
.map...
That way we don't have to use .length later? Do we expect many "empty" strings?
Should this function throw when one of the segments is not valid, instead of attempting to fix the URL?
Contributor
Author
There was a problem hiding this comment.
I think this should just do the logical thing when handed any string?, and not throw. I don't want to add extra error handling to this function. I can pre-filter out falsey strings, but I still would need to do the .length filter afterward in case a string was just a slash, and it only became empty after the map function.
Member
There was a problem hiding this comment.
If we're using length to validate, maybe also "trim" beforehand, so " " is not a valid string?
| </script> | ||
| <script> | ||
| _flutter.buildConfig = { | ||
| useLocalCanvaskit: true, |
eyebrowsoffire
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 7, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 7, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 7, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 8, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jun 8, 2024
…149944) flutter/engine@1cdbebe...45cf05f 2024-06-07 [email protected] [web] add test for inefficient overlay allocation (flutter/engine#53284) 2024-06-07 [email protected] Update Chrome to 125. (flutter/engine#53282) 2024-06-07 [email protected] Roll Dart SDK from 4b693b16eec1 to d4f17e0bf28b (1 revision) (flutter/engine#53283) 2024-06-07 [email protected] Fixes a few issues in flutter_js (flutter/engine#53231) 2024-06-07 [email protected] Roll Fuchsia Linux SDK from aVohW_hnfDaE0smBX... to zpBZmUB_JC5AjG-f4... (flutter/engine#53279) 2024-06-07 [email protected] Roll Dart SDK from 7b239d7f4578 to 4b693b16eec1 (1 revision) (flutter/engine#53277) 2024-06-07 [email protected] Roll Skia from b7f51dfcc342 to ad3c9f203f4e (1 revision) (flutter/engine#53276) 2024-06-07 [email protected] Roll Skia from 5fc83884a619 to b7f51dfcc342 (4 revisions) (flutter/engine#53275) 2024-06-07 [email protected] Revert "Widen CPU affinity set." (flutter/engine#53274) 2024-06-07 [email protected] Roll Skia from e36110f8b451 to 5fc83884a619 (1 revision) (flutter/engine#53273) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from aVohW_hnfDaE to zpBZmUB_JC5A If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot
added a commit
to flutter/flutter
that referenced
this pull request
Jun 8, 2024
…visions) (#149944)" (#149960) Reverts: #149944 Initiated by: jason-simmons Reason for reverting: Regression affecting `integration_test/test/web_extension_test.dart` ``` PathNotFoundException: Cannot open file, path = '/chromium/canvaskit.wasm' (OS Error: No such file or directory, errno = 2) ``` (see https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8745735238400976001/+/u/run_test.dart_for_web_long_running_tests_shard_and_subshard_1_5/stdout) It looks like the p Original PR Author: engine-flutter-autoroll Reviewed By: {fluttergithubbot} This change reverts the following previous change: flutter/engine@1cdbebe...45cf05f 2024-06-07 [email protected] [web] add test for inefficient overlay allocation (flutter/engine#53284) 2024-06-07 [email protected] Update Chrome to 125. (flutter/engine#53282) 2024-06-07 [email protected] Roll Dart SDK from 4b693b16eec1 to d4f17e0bf28b (1 revision) (flutter/engine#53283) 2024-06-07 [email protected] Fixes a few issues in flutter_js (flutter/engine#53231) 2024-06-07 [email protected] Roll Fuchsia Linux SDK from aVohW_hnfDaE0smBX... to zpBZmUB_JC5AjG-f4... (flutter/engine#53279) 2024-06-07 [email protected] Roll Dart SDK from 7b239d7f4578 to 4b693b16eec1 (1 revision) (flutter/engine#53277) 2024-06-07 [email protected] Roll Skia from b7f51dfcc342 to ad3c9f203f4e (1 revision) (flutter/engine#53276) 2024-06-07 [email protected] Roll Skia from 5fc83884a619 to b7f51dfcc342 (4 revisions) (flutter/engine#53275) 2024-06-07 [email protected] Revert "Widen CPU affinity set." (flutter/engine#53274) 2024-06-07 [email protected] Roll Skia from e36110f8b451 to 5fc83884a619 (1 revision) (flutter/engine#53273) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from aVohW_hnfDaE to zpBZmUB_JC5A If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 11, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses a couple things:
flutter/flutter#147610 (Treat
autorenderer properly)flutter/flutter#149443 (Add an entrypoint base url config option)
This also adds a
useLocalCanvasKitconfiguration option on the build config, which the flutter tool can use to fix flutter/flutter#148713