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

[esm-integration] Allow for module argument processing #24005

Merged
merged 1 commit into from
Apr 7, 2025

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Mar 28, 2025

This change delays module argument processing until the init function is run. This means that at least some INCOMING_MODULE_JS_API properties can be supported with ESM integration.

See #24060

@sbc100 sbc100 requested a review from brendandahl March 28, 2025 00:30
@sbc100 sbc100 force-pushed the esm_integration_part2 branch from 53e007d to 7251088 Compare March 28, 2025 00:32
@sbc100 sbc100 force-pushed the esm_integration_part2 branch 2 times, most recently from 9d1afda to a5cfb78 Compare March 31, 2025 23:37
@sbc100
Copy link
Collaborator Author

sbc100 commented Mar 31, 2025

Updated. PTAL

@sbc100 sbc100 force-pushed the esm_integration_part2 branch 2 times, most recently from b49e7c0 to 328d4c0 Compare March 31, 2025 23:56
@sbc100
Copy link
Collaborator Author

sbc100 commented Mar 31, 2025

Done

@sbc100 sbc100 force-pushed the esm_integration_part2 branch from 328d4c0 to 22b0194 Compare April 1, 2025 18:29
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 1, 2025
These are hindering work on other PRs such as emscripten-core#24005.  I've tried a
few other ways to unify and/or change these but simply removing this
mechanism and simplifying things seems like the cleanest approach.

All of the API for which were attaching this assertion have been removed
for well over a year so I think the loss is very minimal.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 1, 2025
These are hindering work on other PRs such as emscripten-core#24005.  I've tried a
few other ways to unify and/or change these but simply removing this
mechanism and simplifying things seems like the cleanest approach.

All of the API for which were attaching this assertion have been removed
for well over a year so I think the loss is very minimal.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 1, 2025
These are hindering work on other PRs such as emscripten-core#24005.  I've tried a
few other ways to unify and/or change these but simply removing this
mechanism and simplifying things seems like the cleanest approach.

All of the API for which were attaching this assertion have been removed
for well over a year so I think the loss is very minimal.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 1, 2025
These are hindering work on other PRs such as emscripten-core#24005.  I've tried a
few other ways to unify and/or change these but simply removing this
mechanism and simplifying things seems like the cleanest approach.

All of the API for which were attaching this assertion have been removed
for well over a year so I think the loss is very minimal.
sbc100 added a commit that referenced this pull request Apr 2, 2025
These are hindering work on other PRs such as #24005. I've tried a few
other ways to unify and/or change these but simply removing this
mechanism and simplifying things seems like the cleanest approach.

All of the API for which were attaching this assertion have been removed
for well over a year so I think the loss is very minimal.
@sbc100 sbc100 force-pushed the esm_integration_part2 branch from 22b0194 to 2e22a8d Compare April 2, 2025 20:14
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 2, 2025
This is in preparation for emscripten-core#24005 which delays the processing and
creation of Module object itself until later in some cases.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 2, 2025
This is in preparation for emscripten-core#24005 which delays the processing and
creation of Module object itself until later in some cases.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 2, 2025
This is in preparation for emscripten-core#24005 which delays the processing and
creation of Module object itself until later in some cases.
sbc100 added a commit that referenced this pull request Apr 2, 2025
…#24039)

This is in preparation for #24005 which delays the processing and
creation of Module object itself until later in some cases.
@sbc100 sbc100 force-pushed the esm_integration_part2 branch 6 times, most recently from faa7609 to d573060 Compare April 3, 2025 21:57
@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 3, 2025

I had to refactor again. I ended up going back to something more like the naming used in your original PR @brendandahl.

Also, I had to introduce another JS file called postlibrary.js which gets inject after the JS library code but before the module creation code.

@sbc100 sbc100 force-pushed the esm_integration_part2 branch 4 times, most recently from 1b3845c to 22bdeae Compare April 7, 2025 21:35
This change delays module argument processing until the init function
is run.  This means that at least some INCOMING_MODULE_JS_API properties
can be supported with ESM integration.

See emscripten-core#24060
@sbc100 sbc100 force-pushed the esm_integration_part2 branch from 22bdeae to 32ba9c8 Compare April 7, 2025 22:05
@sbc100 sbc100 merged commit 14e4fd7 into emscripten-core:main Apr 7, 2025
25 of 28 checks passed
@sbc100 sbc100 deleted the esm_integration_part2 branch April 7, 2025 23:04
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.

2 participants