This repository has been archived by the owner on Oct 17, 2024. It is now read-only.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: surprised that the
dart.library.js
bit was still being chosen by wasm. Asking @srujzs about this now!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dart.library.js
exists inlibraries.json
for dart2wasm. It was used to supportallowInterop
, but that method has since then moved out of that library, so we should delete it fromlibraries.json
. You can't import it (you get a static error), but it exists.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
("Can't import" should match the value of
platform.library.js
, "being there" but not being able to import anyway isn't what a conditional import wants to know.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW - the libraries.json format supports that middleground @lrhn is describing: where you can import a library unconditionally, but the
dart.library.xyz
returns false on conditional imports. This is true today fordart.library.io
on the JS backends, for instance.So if there is any need to still have a library, but denote it as excluded, we can do so with the same mechanism. (This is what ["supported: false"][1] is for)
[1]: https://github.com/dart-lang/sdk/blob/95cbc6a49724f9aaac6ba72fc69a14e9f9f53d70/sdk/lib/libraries.yaml#L269
That said, as mentioned in https://github.com/dart-lang/crypto/issues/180. I believe this specific case can be addressed for now using the
dart.library.ffi
condition.