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

Feat: Support module.uri in AMD extra #2485

Merged
merged 2 commits into from Apr 27, 2024

Conversation

jackw
Copy link
Contributor

@jackw jackw commented Apr 18, 2024

AMD modules loaded via SystemJS should have some way of knowing where they are loading from so they can load their assets. This is mentioned in the requirejs docs here. With this an amd webpack build can then use it to setup __webpack_public_path__. e.g.

__webpack_public_path__ = module.uri.slice(0, module.uri.lastIndexOf('/') + 1);

Fixes: #2386

Notes for reviewers:
Should I update version and run the build script in this PR to update the dist files or does that happen via github-actions after a merge to main?

Copy link

github-actions bot commented Apr 18, 2024

File size impact

Merging jackw/amd-module-uri into main will impact 4 files in 3 groups.

browser (2/2)
File raw gzip brotli Event
dist/s.min.js -25 (7,882) -10 (3,123) -12 (2,833) modified
dist/system.min.js -25 (12,231) -13 (4,731) -10 (4,262) modified
Total size impact -50 (20,113) -23 (7,854) -22 (7,095)
node (1/1)
File raw gzip brotli Event
dist/system-node.cjs -4,040 (517,909) -1,105 (125,031) -935 (84,016) modified
Total size impact -4,040 (517,909) -1,105 (125,031) -935 (84,016)
extras (1/8)
File raw gzip brotli Event
dist/extras/amd.min.js +19 (1,307) +7 (705) +18 (615) modified
Total size impact +19 (1,307) +7 (705) +18 (615)
Generated by file size impact during size-impact#8772233861

@guybedford
Copy link
Member

Sure I would be happy to land and release this. Can you please add a test for this first?

@jackw
Copy link
Contributor Author

jackw commented Apr 21, 2024

@guybedford thanks for taking a look and for the feedback. I've now added a test case for module.uri. Let me know if there are any further changes needed here.

@guybedford guybedford merged commit 1f76d7d into systemjs:main Apr 27, 2024
2 checks passed
@jackw jackw deleted the jackw/amd-module-uri branch April 29, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose module.uri for AMD modules
2 participants