Backport typescript fixes from #2555 to v6.5.0 #2600
Conversation
tsconfig.shared.json
Outdated
| @@ -1,10 +1,10 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "target": "es5", | |||
| "module": "commonjs", | |||
| "target": "es2017", | |||
There was a problem hiding this comment.
Do I need to bring down to es5? I've taken this over from the master branch, but I'm not sure about the implications.
There was a problem hiding this comment.
This should be fine. Where we landed was only the CDN bundle (in @turf/turf) needs to be es5 so it can be imported directly into the browser.
RobinVdBroeck
force-pushed
the
backport-ts-fix
branch
from
44a9ee6
to
de5bb32
Compare
| tsconfig: "tsconfig.json", | ||
| keepNames: true, | ||
| // treeshake: true, causes "chunk.default" warning, breaks CJS exports? | ||
| cjsInterop: false, // putting this to true will break backwards compatability |
There was a problem hiding this comment.
Currently, when importing require("@turf/foo"), you will get back an object containing a property called default, which is the default export. If the property above to false, this will not be the case and the default export will be the actual export if there is only 1 export. However, this is a breaking change, so I had to but this to false.
| options: { | ||
| scripts: { | ||
| "test:tape": "ts-node -r esm test.js", | ||
| bench: "tsx bench.js", |
There was a problem hiding this comment.
Everything is handled by tsx now. I tried to get this to work with -r esm, but I got really weird errors.
RobinVdBroeck
changed the title
| @@ -5,6 +5,6 @@ | |||
| "declaration": true, | |||
| "esModuleInterop": true, | |||
| "strict": true, | |||
| "moduleResolution": "node" | |||
| "moduleResolution": "bundler" | |||
There was a problem hiding this comment.
Since we bundle our code for the consumers of this library, "bundler" is what we need. It uses the module resolution algorithm of node16 without requiring the file extensions.
|
Hey, this PR has been open for a while. Is there any way I can help accelerate merging this? |
|
Hi @RobinVdBroeck. Thanks for submitting the PR. Happy to work with you to get it merged. I do agree with your there is value in backporting some fixes to the v6 branch as well. Please be aware though that all our time and energy at the moment is dedicated to getting v7 out. To cover off an additional testing step, have you tried using arethetypeswrong to check the generated JS and typedefs? |
|
arethetypeswrong report no problems, both for the bundle in 
|
|
Hi, Can we expect a new release with thoses changes soon? Thanks, |
Please fill in this template.
npm testat the sub modules where changes have occurred.npm run lintto ensure code style at the turf module level.Doing an upgrade to 7.0.0 might be a big endeavor for some users of this library, so this would allow them to use modern Typescript features (like node16 module resolution) or use a bundler like vite.
I've backported the changes done in #2555 to v6.5.0. I haven't backported the pnpm changes, since that seems to be out of scope for this backport.
I've bumped the version of typescript here to 4.7.2, since that is the first one to introduce the
node16module resolution algorithm.I've manually verified the changes by setting up a minimal vite project, which was previously incompatible with v6.5.0, and using my local build (by yarn link). I've also setup a small script (
const foo = require("@turf/union").default; console.log(foo != undefined) to make sure that commonjs is not broken.tsup requires that there is a
tsconfig.jsonfile in every package, so I've added a scriptsync-tsconfigsthat writes the tsconfig.json to every package.