-
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add support for TLA on node 14.3 and newer #7
Conversation
@@ -45,8 +45,13 @@ for (const [name, cases] of Object.entries(tests)) { | |||
// runtime. It is supported starting from 10.4, so we can check the version. | |||
const major = parseInt(process.versions.node, 10); | |||
const minor = parseInt(process.versions.node.match(/^\d+\.(\d+)/)[1], 10); | |||
if (major > 10 || (major === 10 && minor > 4)) { | |||
if (major > 10 || (major === 10 && minor >= 4)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug fix looking at the comment above
@@ -15,7 +15,7 @@ jobs: | |||
|
|||
strategy: | |||
matrix: | |||
node-version: [10.19, 12.4, 12.8, 13.11, 14] | |||
node-version: [10.19, 12.4, 12.8, 13.11, 14.3, 14, 15] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess 15 doesn't matter, but anyways
For the tests, maybe passing
|
|
There's something else different between the Jest repo and this repo. If I don't have the syntax plugin in the preset in the Jest repo, I get this error
But the error I get in this repo - with or without the syntax plugin - is
I would have thought just using a newer version of babel.parseSync(code, {
configFile: false,
presets: [thisPreset],
sourceType: "module",
caller: {
name: "syntax-preset-tester",
supportsStaticESM: true,
supportsDynamicImport: true,
supportsTopLevelAwait: true,
supportsExportNamespaceFrom: true,
},
}) |
Oh I checked the lockfile and it's still using |
Ah, I missed the resolutions entry! Unlocking EDIT: Just removing the resolution work, the dev dep doesn't matter |
Ok so, the problem is that for the other tests we still need @arcanis If I have this package.json "devDependencies": {
"@babel/core@7.0.0": "npm:@babel/core@7.0.0",
"@babel/core@7.9.0": "npm:@babel/core@7.9.0"
}, Can I use |
I think I found a fix. |
Tests are passing ✌️ |
Yay! 🎉 |
I'll release 1.0.0, since this project is "stable enough" |
wonderful, thanks! |
Bumped in jestjs/jest#10747, will release in v26 either this weekend or monday |
See babel/babel#12292
https://nodejs.org/en/blog/release/v14.3.0/
I'm unable to make this pass the tests, tho :( Weirdly, copying in the code here makes TLA work in a test in Jest repo, but I keep getting parser errors here. I tried updating
@babel/core
to latest just in case, but that didn't make a difference.One interesting observation is that in Jest it only works if I use
loadPartialConfig
(which is what we use by default) - if I useloadOptions
instead it fails with the same error it fails with here. However, I'm not having any luck replicating that behavior in this preset