feat(compartment-mapper): Thread commonjs- and module- languageForExtension options (for TS) #2625
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
changed the title
kriskowal
force-pushed
the
kriskowal-compartment-mapper-d-ts
branch
from
071e61e to
6d146a4
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
caffdaa to
0a25959
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-d-ts
branch
from
6d146a4 to
905d9da
Compare
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
0a25959 to
42b8b18
Compare
kriskowal
commented
Nov 8, 2024
| errorTrapping: 'none', | ||
| }); | ||
|
|
||
| scaffold( |
There was a problem hiding this comment.
For reviewers new to Compartment Mapper, scaffold generates a combination of test cases where a single fixture is expected to produce the same behavior for multiple flows:
- executed directly off a file system
- loaded off the file system then later executed
- archived off the file system, then parsed, then executed
&c
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
5 times, most recently
from
f77329b to
d6ce6de
Compare
turadg
approved these changes
Nov 9, 2024
| mjs: 'mjs', | ||
| cjs: 'cjs', |
There was a problem hiding this comment.
consider alphabetizing or leaving where it was
| ); | ||
| } | ||
| const invalidLanguages = values(packageLanguageForExtension).filter( | ||
| language => !languages.has(language), |
There was a problem hiding this comment.
language and languages look very similar. consider,
Suggested change
| language => !languages.has(language), | |
| l => !languages.has(l), |
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
6 times, most recently
from
60b295c to
300f46c
Compare
…ized on package type
kriskowal
force-pushed
the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
from
300f46c to
389de7b
Compare
kriskowal
deleted the
kriskowal-compartment-mapper-module-vs-commonjs-options
branch
kriskowal
added a commit
that referenced
this pull request
Nov 12, 2024
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 `.cts` extensions in packages that are not under `node_modules`, to discourage publishing TypeScript as a source language to npm. ### Security Considerations The choice of `ts-blank-space` is 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.
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.
Refs: #2415
Description
Toward support for type-erasure style TypeScript support in Endo,
.tsfiles may correspond to.mtsor.ctsbehavior in the same way that.jscan correspond to either.mjsor.cjs, depending on"type":in the envelopingpackage.json(which is not necessarily the enveloping package, e.g.,src/package.jsonbeneathpackage.json!). We already implement this machinery for JavaScript, so this change threads additional options besidelanguageForExtension(which would map.mtsand.ctsto language behaviors) but alsocommonjsLanguageForExtensionandmoduleLanguageForExtension, which would get folded into the language-for-extension mapping on a package-by-package basis.Security Considerations
None. Absent from this design is any possibility that a file would be alternately implemented as TypeScript or JavaScript.
Scaling Considerations
None.
Documentation Considerations
Includes an update to README for the new features and the undocumented features it is based upon. Includes a mention in NEWS.
Testing Considerations
TODO
Compatibility Considerations
By design error,
languageForExtensionthreaded throughlinkbefore this change, when it should have been threaded throughmapNodeModules, since that is the machine that is aware of thetypefield inpackage.json. This change moves that internal mechanism and then addscommonjsLanguageForExtensionandmoduleLanguageForExtension. By common usage, this move is transparent since the option is taken at the top and simply diverted internally.However, the
languageForExtensionproperty could be manually threaded throughmakeArchiveLiteand omitted from the options formapNodeModules. I’ve elected to treat this breaking change as a bug fix since I find it unlikely anyone has taken advantage of these relatively new “lite” functions. The “lite” functions are part of the refactoring necessary to approach native XS compartment and modules.Upgrade Considerations
None.