Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for using the Frontend Server with the Legacy/DDC modu… #2285

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

Markzipan
Copy link
Contributor

This change adds a Frontend Server + Legacy module system strategy provider and adds some additional logic to the legacy bootstrapper. This logic interfaces with some of the API in dart_library.js as well as in the bootstrapper itself.

This is part of a greater effort to deprecate the AMD module system: dart-lang/sdk#52361

@Markzipan
Copy link
Contributor Author

@annagrin @elliette Can you all point me to some places where I can add/plumb through tests + changelog/versioning details?

@Markzipan Markzipan force-pushed the legacy-support branch 4 times, most recently from 4e3cdb1 to 8490a07 Compare November 8, 2023 01:32
@annagrin
Copy link
Contributor

annagrin commented Nov 8, 2023

Thanks Mark!

  • please update the change log similar to in https://github.com/dart-lang/webdev/blob/master/dwds/CHANGELOG.md?plain=1#L2 (you can also see other PRs for examples).

  • since you didn't change the API you don't need to update a version, so you can just add to the current version changelog

  • This can be done in a separate PR if needed: please add a test option in context.dart for module format that allows to use the new strategy with the frontend server compilation mode and add a test (you can just copy and modify tests/frontend_server_evaluate_sound_test.dart).

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Mark, I left some comments.

dwds/lib/src/loaders/frontend_server_legacy.dart Outdated Show resolved Hide resolved
@Markzipan
Copy link
Contributor Author

Thanks Anna! Comments addressed + CHANGELOG entry added. I can land the tests in a followup.

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Markzipan Markzipan merged commit e67071c into dart-lang:master Nov 9, 2023
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants