Skip to content

Feat: Support module.uri in AMD extra#2485

Merged
guybedford merged 2 commits intosystemjs:mainfrom
jackw:jackw/amd-module-uri
Apr 27, 2024
Merged

Feat: Support module.uri in AMD extra#2485
guybedford merged 2 commits intosystemjs:mainfrom
jackw:jackw/amd-module-uri

Conversation

@jackw
Copy link
Copy Markdown
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?

@github-actions
Copy link
Copy Markdown

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
Copy Markdown
Member

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

@jackw
Copy link
Copy Markdown
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.

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