diff --git a/ChangeLog.md b/ChangeLog.md index aedfee2d08372..806581db20faa 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -22,8 +22,10 @@ Current Trunk will need to wait on the returned Promise, using `await` or its `then` callback, to get the module instance (#10697). This fixes some long-standing bugs with that option which have been reported multiple times, but is a - breaking change - sorry about that. See detailed examples for the - current usage in `src/settings.js` on `MODULARIZE`. + breaking change - sorry about that. To reduce the risk of confusing breakage, + in a build with `ASSERTIONS` we will show a clear warning on common errors. + For more, see detailed examples for the current usage in `src/settings.js` on + `MODULARIZE`. - A new `PRINTF_LONG_DOUBLE` option allows printf to print long doubles at full float128 precision. (#11130) - `emscripten_async_queue_on_thread` has been renamed to diff --git a/src/parseTools.js b/src/parseTools.js index b493280b2e5ef..26455d426c56d 100644 --- a/src/parseTools.js +++ b/src/parseTools.js @@ -1708,3 +1708,26 @@ function sendI64Argument(low, high) { return low + ', ' + high; } } + +// Add assertions to catch common errors when using the Promise object we +// create on Module.ready() and return from MODULARIZE Module() invocations. +function addReadyPromiseAssertions(promise) { + // Warn on someone doing + // + // var instance = Module(); + // ... + // instance._main(); + var properties = keys(EXPORTED_FUNCTIONS); + // Also warn on onRuntimeInitialized which might be a common pattern with + // older MODULARIZE-using codebases. + properties.push('onRuntimeInitialized'); + return properties.map(function(property) { + const warningEnding = `${property} on the Promise object, instead of the instance. Use .then() to get called back with the instance, see the MODULARIZE docs in src/settings.js`; + return ` + if (!Object.getOwnPropertyDescriptor(${promise}, '${property}')) { + Object.defineProperty(${promise}, '${property}', { configurable: true, get: function() { abort('You are getting ${warningEnding}') } }); + Object.defineProperty(${promise}, '${property}', { configurable: true, set: function() { abort('You are setting ${warningEnding}') } }); + } + `; + }).join('\n'); +} diff --git a/src/shell.js b/src/shell.js index d820d97fe4d3a..8a3f1994d8979 100644 --- a/src/shell.js +++ b/src/shell.js @@ -45,6 +45,9 @@ Module['ready'] = new Promise(function(resolve, reject) { readyPromiseResolve = resolve; readyPromiseReject = reject; }); +#if ASSERTIONS +{{{ addReadyPromiseAssertions("Module['ready']") }}} +#endif #endif // --pre-jses are emitted after the Module integration code, so that they can diff --git a/src/shell_minimal.js b/src/shell_minimal.js index 0ab58587447fd..4f62340357253 100644 --- a/src/shell_minimal.js +++ b/src/shell_minimal.js @@ -22,6 +22,9 @@ Module['ready'] = new Promise(function(resolve, reject) { readyPromiseResolve = resolve; readyPromiseReject = reject; }); +#if ASSERTIONS +{{{ addReadyPromiseAssertions("Module['ready']") }}} +#endif #endif #if ENVIRONMENT_MAY_BE_NODE diff --git a/tests/test_other.py b/tests/test_other.py index c281d46722687..06c2426fe056e 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -10386,6 +10386,29 @@ def test_assertions_on_outgoing_module_api_changes(self): Module.arguments has been replaced with plain arguments_ ''', run_js('a.out.js', assert_returncode=None, stderr=PIPE)) + def test_assertions_on_ready_promise(self): + # check that when assertions are on we give useful error messages for + # mistakenly thinking the Promise is an instance. I.e., once you could do + # Module()._main to get an instance and the main function, but after + # the breaking change in #10697 Module() now returns a promise, and to get + # the instance you must use .then() to get a callback with the instance. + create_test_file('test.js', r''' + try { + Module()._main; + } catch(e) { + console.log(e); + } + try { + Module().onRuntimeInitialized = 42; + } catch(e) { + console.log(e); + } + ''') + run_process([PYTHON, EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'MODULARIZE', '-s', 'ASSERTIONS', '--extern-post-js', 'test.js']) + out = run_js('a.out.js') + self.assertContained('You are getting _main on the Promise object, instead of the instance. Use .then() to get called back with the instance, see the MODULARIZE docs in src/settings.js', out) + self.assertContained('You are setting onRuntimeInitialized on the Promise object, instead of the instance. Use .then() to get called back with the instance, see the MODULARIZE docs in src/settings.js', out) + def test_em_asm_duplicate_strings(self): # We had a regression where tow different EM_ASM strings from two diffferent # object files were de-duplicated in wasm-emscripten-finalize. This used to