Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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?

@github-actions
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.

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.