[tsgen] Make commonjs module output compatible with tsc. #22988
Conversation
This enables commonjs modules to be imported into TypeScript using the same syntax as ESM modules e.g. `import moduleFactory from './embind_tsgen.js';`
brendandahl
force-pushed
the
tsgen-commonjs
branch
from
033cd49
to
88463f9
Compare
| module.exports = %(EXPORT_NAME)s; | ||
| else if (typeof define === 'function' && define['amd']) | ||
| // This default export allows TS to import this commonjs style module. | ||
| module.exports.default = %(EXPORT_NAME)s; |
There was a problem hiding this comment.
Do we both of these though? It seems a little odd..
There was a problem hiding this comment.
Yes, for the module to work in both node and TypeScript. There's a whole article on why it's hard to support both https://www.typescriptlang.org/docs/handbook/modules/appendices/esm-cjs-interop.html
There was a problem hiding this comment.
I wonder if its worth include that link here, or elaborating on the comment. e.g. "This might seem redundant but without this it is not possible to ...".
| @@ -1,30 +1,36 @@ | |||
| // Example TS program that consumes the emscripten-generated module to to | |||
| // illustrate how the type definitions are used and test they are workings as | |||
| // expected. | |||
| import moduleFactory from './embind_tsgen.mjs'; | |||
| import moduleFactory from './embind_tsgen.js'; | |||
There was a problem hiding this comment.
Should we not be testing both of these options?
There was a problem hiding this comment.
ESM and commonjs are still both tested, package.json determines if it's a one or the other.
There was a problem hiding this comment.
I see.. so this .js file can be either one, depending on the test?
Perhaps add a comment to that effect here?
| @@ -3385,21 +3385,24 @@ def test_jspi_add_function(self): | |||
| self.do_runf('other/test_jspi_add_function.c', 'done') | |||
|
|
|||
| @parameterized({ | |||
| '': [[]], | |||
| 'with_jsgen': [['-sEMBIND_AOT']] | |||
| 'commonjs': [['-sMODULARIZE'], ['--module', 'commonjs', '--moduleResolution', 'node']], | |||
There was a problem hiding this comment.
FYI: this roughly matches how emscripten output is used in g3.
This enables commonjs modules to be imported into TypeScript using the same syntax as ESM modules e.g.
import moduleFactory from './embind_tsgen.js';