Skip to content

Commit dcb4112

Browse files
authored
Add useful errors on bad use of Promise instead of instance in new MODULARIZE (#11118)
It used to be possible to do Module().onRuntimeInitialized = ... but with new MODULARIZE (#10697) Module() returns a Promise instead. The Promise of course allows a property to be placed on it, but nothing would happen, so it's potentially annoying for users. With this change, when ASSERTIONS are on we will throw an error with an explanation, something like 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 Likewise, it used to be possible to do var instance = Module(); // .. later .. instance.exportedThing() but again, the "instance" is a Promise now, and the exports aren't there. This will crash, but ".. is not a function" is not that helpful. With this change it will give an explanation of what's wrong.
1 parent 52d82b8 commit dcb4112

File tree

5 files changed

+56
-2
lines changed

5 files changed

+56
-2
lines changed

ChangeLog.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@ Current Trunk
2626
will need to wait on the returned Promise, using `await` or its `then`
2727
callback, to get the module instance (#10697). This fixes some long-standing
2828
bugs with that option which have been reported multiple times, but is a
29-
breaking change - sorry about that. See detailed examples for the
30-
current usage in `src/settings.js` on `MODULARIZE`.
29+
breaking change - sorry about that. To reduce the risk of confusing breakage,
30+
in a build with `ASSERTIONS` we will show a clear warning on common errors.
31+
For more, see detailed examples for the current usage in `src/settings.js` on
32+
`MODULARIZE`.
3133
- A new `PRINTF_LONG_DOUBLE` option allows printf to print long doubles at full
3234
float128 precision. (#11130)
3335
- `emscripten_async_queue_on_thread` has been renamed to

src/parseTools.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,3 +1708,26 @@ function sendI64Argument(low, high) {
17081708
return low + ', ' + high;
17091709
}
17101710
}
1711+
1712+
// Add assertions to catch common errors when using the Promise object we
1713+
// create on Module.ready() and return from MODULARIZE Module() invocations.
1714+
function addReadyPromiseAssertions(promise) {
1715+
// Warn on someone doing
1716+
//
1717+
// var instance = Module();
1718+
// ...
1719+
// instance._main();
1720+
var properties = keys(EXPORTED_FUNCTIONS);
1721+
// Also warn on onRuntimeInitialized which might be a common pattern with
1722+
// older MODULARIZE-using codebases.
1723+
properties.push('onRuntimeInitialized');
1724+
return properties.map(function(property) {
1725+
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`;
1726+
return `
1727+
if (!Object.getOwnPropertyDescriptor(${promise}, '${property}')) {
1728+
Object.defineProperty(${promise}, '${property}', { configurable: true, get: function() { abort('You are getting ${warningEnding}') } });
1729+
Object.defineProperty(${promise}, '${property}', { configurable: true, set: function() { abort('You are setting ${warningEnding}') } });
1730+
}
1731+
`;
1732+
}).join('\n');
1733+
}

src/shell.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ Module['ready'] = new Promise(function(resolve, reject) {
4545
readyPromiseResolve = resolve;
4646
readyPromiseReject = reject;
4747
});
48+
#if ASSERTIONS
49+
{{{ addReadyPromiseAssertions("Module['ready']") }}}
50+
#endif
4851
#endif
4952

5053
// --pre-jses are emitted after the Module integration code, so that they can

src/shell_minimal.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ Module['ready'] = new Promise(function(resolve, reject) {
2222
readyPromiseResolve = resolve;
2323
readyPromiseReject = reject;
2424
});
25+
#if ASSERTIONS
26+
{{{ addReadyPromiseAssertions("Module['ready']") }}}
27+
#endif
2528
#endif
2629

2730
#if ENVIRONMENT_MAY_BE_NODE

tests/test_other.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10402,6 +10402,29 @@ def test_assertions_on_outgoing_module_api_changes(self):
1040210402
Module.arguments has been replaced with plain arguments_
1040310403
''', run_js('a.out.js', assert_returncode=None, stderr=PIPE))
1040410404

10405+
def test_assertions_on_ready_promise(self):
10406+
# check that when assertions are on we give useful error messages for
10407+
# mistakenly thinking the Promise is an instance. I.e., once you could do
10408+
# Module()._main to get an instance and the main function, but after
10409+
# the breaking change in #10697 Module() now returns a promise, and to get
10410+
# the instance you must use .then() to get a callback with the instance.
10411+
create_test_file('test.js', r'''
10412+
try {
10413+
Module()._main;
10414+
} catch(e) {
10415+
console.log(e);
10416+
}
10417+
try {
10418+
Module().onRuntimeInitialized = 42;
10419+
} catch(e) {
10420+
console.log(e);
10421+
}
10422+
''')
10423+
run_process([PYTHON, EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'MODULARIZE', '-s', 'ASSERTIONS', '--extern-post-js', 'test.js'])
10424+
out = run_js('a.out.js')
10425+
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)
10426+
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)
10427+
1040510428
def test_em_asm_duplicate_strings(self):
1040610429
# We had a regression where tow different EM_ASM strings from two diffferent
1040710430
# object files were de-duplicated in wasm-emscripten-finalize. This used to

0 commit comments

Comments
 (0)