feat(bundle-source): Support TypeScript type erasure #2627
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
d6ce6de
to
f156aea
Compare
kriskowal
force-pushed
the
kriskowal-ts
branch
2 times, most recently
from
18f730e
to
6b08628
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
f156aea
to
ff2ec1b
Compare
turadg
reviewed
Nov 9, 2024
packages/bundle-source/README.md
Outdated
Comment on lines
70
to
71
| This will also not function properly for TypeScript modules that generate | ||
| JavaScript runtime code, like enums. |
There was a problem hiding this comment.
"enums" aren't necessarily TypeScript. (JSDoc has @enum).
Suggested change
| This will also not function properly for TypeScript modules that generate | |
| JavaScript runtime code, like enums. | |
| This will also not function properly for TypeScript modules that have [runtime impacting syntax](https://github.com/bloomberg/ts-blank-space/blob/main/docs/unsupported_syntax.md), such as `enum`. |
| t.is(bundle.moduleFormat, 'endoScript'); | ||
| const { source } = bundle; | ||
| const compartment = new Compartment(); | ||
| const ns = compartment.evaluate(source); | ||
| t.is(ns.meaning, 42); | ||
| }); | ||
|
|
||
| test('endo script format supports typescript type erasure', async t => { |
There was a problem hiding this comment.
I think we need more test cases to help make clear what to expect.
IIUC this these work:
// tsFromTs.ts
export { fortune } from './fortune.ts// tsFromJs.ts
export { meaning } from './meaning.jsAnd this won't,
// jsFromTs.js
export { fortune } from './fortune.tsThere was a problem hiding this comment.
I’ve posted tests alongside these that verify that the full cross-product of {js,ts} can import and export {js,ts}, and also an expected failing test that you can’t import 'fortune.js' in TypeScript, if there is no fortune.js, even if there is fortune.ts.
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
ff2ec1b
to
db2ad9a
Compare
kriskowal
force-pushed
the
kriskowal-ts
branch
from
6b08628
to
5f345d5
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
db2ad9a
to
b1719f8
Compare
kriskowal
force-pushed
the
kriskowal-ts
branch
from
5f345d5
to
06c3b19
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
b1719f8
to
60b295c
Compare
kriskowal
force-pushed
the
kriskowal-ts
branch
2 times, most recently
from
fe0022a
to
2296b4f
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
60b295c
to
300f46c
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
300f46c
to
389de7b
Compare
kriskowal
force-pushed
the
kriskowal-ts
branch
from
2296b4f
to
6203ef4
Compare
Base automatically changed from
kriskowal-compartment-mapper-module-vs-commonjs-options
to
master
November 12, 2024 01:01
kriskowal
force-pushed
the
kriskowal-ts
branch
from
6203ef4
to
91caef7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #2415
Description
This change introduces support for TypeScript through type-erasure, using ts-blank-space, which converts type annotations to equivalent blank space. As is consistent with
node --experimental-strip-types, this only applies to modules with the.ts,.mts, or.ctsextensions in packages that are not undernode_modules, to discourage publishing TypeScript as a source language to npm.Security Considerations
The choice of
ts-blank-spaceis intended to minimize runtime behavior difference between TypeScript and JavaScript, such that a reviewer or a debugger of the generated JavaScript aligns with the expected behavior and original text, to the extent that is possible. This should compose well with #2444.Scaling Considerations
None.
Documentation Considerations
Contains README and NEWS.
Testing Considerations
Contains spot check tests for TypeScript in the endoScript and endoZipBase64 formats. We stand on much more rigorous testing of the underlying workspace-language-for-extension feature in Compartment Mapper #2625.
Compatibility Considerations
This does not break any prior usage.
Upgrade Considerations
None.