Skip to content

Commit

Permalink
Assert if MODULARIZE factory function is used with new. (#23210)
Browse files Browse the repository at this point in the history
Once we start using `async` for for this function then `new` stops
working: #23157.

Using `new` in this was was always an antipattern but there was nothing
atopping users from doing this.
  • Loading branch information
sbc100 authored Dec 18, 2024
1 parent 79177cc commit 669e01a
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 22 deletions.
5 changes: 5 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ See docs/process.md for more on how version tagging works.

3.1.75 (in development)
-----------------------
- When using `-sMODULARIZE` we now assert if the factory function is called with
the JS `new` keyword. e.g. `a = new Module()` rather than `b = Module()`.
This paves the way for marking the function as `async` which does not allow
`new` to be used. This usage of `new` here was never documented and is
considered and antipattern. (#23210)
- `PATH.basename()` no longer calls `PATH.normalize()`, so that
`PATH.basename("a/.")` returns `"."` instead of `"a"` and
`PATH.basename("a/b/..")` returns `".."` instead of `"a"`. This is in line with
Expand Down
35 changes: 13 additions & 22 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4795,28 +4795,19 @@ def test_browser_run_from_different_directory(self):

# Similar to `test_browser_run_from_different_directory`, but asynchronous because of `-sMODULARIZE`
def test_browser_run_from_different_directory_async(self):
for args, creations in [
(['-sMODULARIZE'], [
'Module();', # documented way for using modularize
'new Module();' # not documented as working, but we support it
]),
]:
print(args)
# compile the code with the modularize feature and the preload-file option enabled
self.compile_btest('browser_test_hello_world.c', ['-o', 'test.js', '-O3'] + args)
ensure_dir('subdir')
shutil.move('test.js', Path('subdir/test.js'))
shutil.move('test.wasm', Path('subdir/test.wasm'))
for creation in creations:
print(creation)
# Make sure JS is loaded from subdirectory
create_file('test-subdir.html', '''
<script src="subdir/test.js"></script>
<script>
%s
</script>
''' % creation)
self.run_browser('test-subdir.html', '/report_result?0')
# compile the code with the modularize feature and the preload-file option enabled
self.compile_btest('browser_test_hello_world.c', ['-o', 'test.js', '-O3', '-sMODULARIZE'])
ensure_dir('subdir')
shutil.move('test.js', Path('subdir/test.js'))
shutil.move('test.wasm', Path('subdir/test.wasm'))
# Make sure JS is loaded from subdirectory
create_file('test-subdir.html', '''
<script src="subdir/test.js"></script>
<script>
Module();
</script>
''')
self.run_browser('test-subdir.html', '/report_result?0')

# Similar to `test_browser_run_from_different_directory`, but
# also also we eval the initial code, so currentScript is not present. That prevents us
Expand Down
6 changes: 6 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -6774,6 +6774,12 @@ def test_modularize_strict(self):
output = self.run_js('run.js')
self.assertEqual(output, 'hello, world!\n')

def test_modularize_new_misuse(self):
self.run_process([EMCC, test_file('hello_world.c'), '-sMODULARIZE', '-sEXPORT_NAME=Foo'])
create_file('run.js', 'var m = require("./a.out.js"); new m();')
err = self.run_js('run.js', assert_returncode=NON_ZERO)
self.assertContained('Error: Foo() should not be called with `new Foo()`', err)

@parameterized({
'': ([],),
'export_name': (['-sEXPORT_NAME=Foo'],),
Expand Down
13 changes: 13 additions & 0 deletions tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -2452,6 +2452,19 @@ def modularize():
'wrapper_function': wrapper_function,
}

if settings.ASSERTIONS and settings.MODULARIZE != 'instance':
src += '''\
(() => {
// Create a small, never-async wrapper around %(EXPORT_NAME)s which
// checks for callers incorrectly using it with `new`.
var real_%(EXPORT_NAME)s = %(EXPORT_NAME)s;
%(EXPORT_NAME)s = function(arg) {
if (new.target) throw new Error("%(EXPORT_NAME)s() should not be called with `new %(EXPORT_NAME)s()`");
return real_%(EXPORT_NAME)s(arg);
}
})();
''' % {'EXPORT_NAME': settings.EXPORT_NAME}

# Given the async nature of how the Module function and Module object
# come into existence in AudioWorkletGlobalScope, store the Module
# function under a different variable name so that AudioWorkletGlobalScope
Expand Down

0 comments on commit 669e01a

Please sign in to comment.