Skip to content

Commit 5dbbd71

Browse files
authored
Avoid exporting "ready" promise on the module object (#21564)
I believe the "ready" promise was set on the Module object here primarily so that closure would not minify the name. Instead of polluting the Module namespace we can instead just tell closure not to minify it. In addition to polluting the `Module` namespace exporting a `ready` promise is also confusing since the only way to get a module object when `-sMODULARIZE` is used is via waiting on the promise returned from the factory function. So, by definition, the promise has already resolved before anyone can access it. Instead we use the `closure-externs.js` file to suppress the minifiction which matches the behavior of the incoming `moduleArg` argument.
1 parent b66b467 commit 5dbbd71

File tree

6 files changed

+31
-17
lines changed

6 files changed

+31
-17
lines changed

src/closure-externs/closure-externs.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,21 @@ var sampleRate;
242242
*/
243243
var id;
244244

245+
/**
246+
* Used in MODULARIZE mode as the name of the incoming module argument.
247+
* This is generated outside of the code we pass to closure so from closure's
248+
* POV this is "extern".
249+
*/
245250
var moduleArg;
246251

252+
/**
253+
* Used in MODULARIZE mode.
254+
* We need to access this after the code we pass to closure so from closure's
255+
* POV this is "extern".
256+
* @suppress {duplicate}
257+
*/
258+
var readyPromise;
259+
247260
/**
248261
* This was removed from upstream closure compiler in
249262
* https://github.com/google/closure-compiler/commit/f83322c1b.

src/parseTools.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -984,8 +984,8 @@ function to64(x) {
984984
}
985985

986986
// Add assertions to catch common errors when using the Promise object we
987-
// create on Module.ready() and return from MODULARIZE Module() invocations.
988-
function addReadyPromiseAssertions(promise) {
987+
// return from MODULARIZE Module() invocations.
988+
function addReadyPromiseAssertions() {
989989
// Warn on someone doing
990990
//
991991
// var instance = Module();
@@ -1001,8 +1001,8 @@ function addReadyPromiseAssertions(promise) {
10011001
return (
10021002
res +
10031003
`.forEach((prop) => {
1004-
if (!Object.getOwnPropertyDescriptor(${promise}, prop)) {
1005-
Object.defineProperty(${promise}, prop, {
1004+
if (!Object.getOwnPropertyDescriptor(readyPromise, prop)) {
1005+
Object.defineProperty(readyPromise, prop, {
10061006
get: () => abort('You are getting ' + prop + '${warningEnding}'),
10071007
set: () => abort('You are setting ' + prop + '${warningEnding}'),
10081008
});

src/settings_internal.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,9 @@ var WASM_EXCEPTIONS = false;
202202
// EXPORTED_FUNCTIONS then this gets set to 0.
203203
var EXPECT_MAIN = true;
204204

205-
// Provide and export a .ready() Promise. This is currently used by default with
206-
// MODULARIZE, and returned from the factory function.
207-
var EXPORT_READY_PROMISE = true;
205+
// Return a "ready" Promise from the MODULARIZE factory function.
206+
// We disable this under some circumstance if we know its not needed.
207+
var USE_READY_PROMISE = true;
208208

209209
// If true, building against Emscripten's wasm heap memory profiler.
210210
var MEMORYPROFILER = false;

src/shell.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ var Module = typeof {{{ EXPORT_NAME }}} != 'undefined' ? {{{ EXPORT_NAME }}} : {
6666
#if MODULARIZE
6767
// Set up the promise that indicates the Module is initialized
6868
var readyPromiseResolve, readyPromiseReject;
69-
Module['ready'] = new Promise((resolve, reject) => {
69+
var readyPromise = new Promise((resolve, reject) => {
7070
readyPromiseResolve = resolve;
7171
readyPromiseReject = reject;
7272
});
7373
#if ASSERTIONS
74-
{{{ addReadyPromiseAssertions("Module['ready']") }}}
74+
{{{ addReadyPromiseAssertions() }}}
7575
#endif
7676
#endif
7777

src/shell_minimal.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ var Module =
3333
var Module = {{{ EXPORT_NAME }}};
3434
#endif
3535

36-
#if MODULARIZE && EXPORT_READY_PROMISE
36+
#if MODULARIZE && USE_READY_PROMISE
3737
// Set up the promise that indicates the Module is initialized
3838
var readyPromiseResolve, readyPromiseReject;
39-
Module['ready'] = new Promise((resolve, reject) => {
39+
var readyPromise = new Promise((resolve, reject) => {
4040
readyPromiseResolve = resolve;
4141
readyPromiseReject = reject;
4242
});
4343
#if ASSERTIONS
44-
{{{ addReadyPromiseAssertions("Module['ready']") }}}
44+
{{{ addReadyPromiseAssertions() }}}
4545
#endif
4646
#endif
4747

@@ -125,7 +125,7 @@ var err = (text) => console.error(text);
125125
// compilation is ready. In that callback, call the function run() to start
126126
// the program.
127127
function ready() {
128-
#if MODULARIZE && EXPORT_READY_PROMISE
128+
#if MODULARIZE && USE_READY_PROMISE
129129
readyPromiseResolve(Module);
130130
#endif // MODULARIZE
131131
#if INVOKE_RUN && HAS_MAIN

tools/link.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,7 +1141,7 @@ def phase_linker_setup(options, state, newargs):
11411141
# Promise. However, in Pthreads mode the Promise is used for worker
11421142
# creation.
11431143
if settings.MINIMAL_RUNTIME and options.oformat == OFormat.HTML and not settings.PTHREADS:
1144-
settings.EXPORT_READY_PROMISE = 0
1144+
settings.USE_READY_PROMISE = 0
11451145

11461146
if settings.WASM2JS and settings.LEGACY_VM_SUPPORT:
11471147
settings.POLYFILL_OLD_MATH_FUNCTIONS = 1
@@ -2334,9 +2334,10 @@ def modularize():
23342334
# the return statement.
23352335
return_value = 'moduleArg'
23362336
if settings.WASM_ASYNC_COMPILATION:
2337-
return_value += '.ready'
2338-
if not settings.EXPORT_READY_PROMISE:
2339-
return_value = '{}'
2337+
if settings.USE_READY_PROMISE:
2338+
return_value = 'readyPromise'
2339+
else:
2340+
return_value = '{}'
23402341

23412342
# TODO: Remove when https://bugs.webkit.org/show_bug.cgi?id=223533 is resolved.
23422343
if async_emit != '' and settings.EXPORT_NAME == 'config':

0 commit comments

Comments
 (0)