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

Avoid defining Module in shell_minimal.js. NFC #22298

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jul 31, 2024

Instead we can just always use the same technique to define Module in shell_minimal.js, regardless of the target environment.

This simplifies the code and reduces code size.

Instead we can just always use the same technique to define `Module`
in `shell_minimal.js`, regardless of the target environment.

This simplifies the code and reduces code size.
@sbc100 sbc100 requested review from juj and kripken July 31, 2024 13:57
@@ -28,9 +27,6 @@ var Module =
// Otherwise do a good old typeof check.
typeof {{{ EXPORT_NAME }}} != 'undefined' ? {{{ EXPORT_NAME }}} : {};
#endif

#else
var Module = {{{ EXPORT_NAME }}};
Copy link
Collaborator

@juj juj Aug 1, 2024

Choose a reason for hiding this comment

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

It looks like this reduces code size only for users who do not need to define Module in .html file at all. (if user does need to define Module in .html, then the original var Module = {{{ EXPORT_NAME }}}; definition would be shorter.

Although I think that is ok.

It may read a bit simpler to further condense the code to

#elif SUPPORTS_GLOBALTHIS
var Module = globalThis.{{{ EXPORT_NAME }}} || {};
#else
var Module = typeof {{{ EXPORT_NAME }}} != 'undefined' ? {{{ EXPORT_NAME }}} : {};
#endif

but works either way.

@sbc100 sbc100 closed this Jan 3, 2025
@sbc100 sbc100 deleted the module_def_minimal branch January 3, 2025 22:45
@sbc100 sbc100 restored the module_def_minimal branch January 4, 2025 00:27
@sbc100 sbc100 reopened this Jan 4, 2025
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.

3 participants