Develop and test as maintainers without building JS first #2988
Conversation
…ete with esnext/bundler upgrade. Produces identical dist/ output (except for a few .cjs.map files) as the current build.
… tsup.config.ts files that include auto-generated project references where required. Several TS and JS specific template files added. Had to stop prettier from checking those files though as the formats clashed. Updated nx dependency to not require package build before test. Added earcut DT types to turf-tesselate in order to retire local earcut.d.ts and keep tsconfig.json generation simple. tsup now refers to per-package tsup.config.ts (and from there per-package tsconfig.build.json) Full list of package paths listed in tsconfig.shared.json (manually maintained at the moment).
… new prerelease top level target. Separated other top level targets to build only what we need for each npm lifecycle phase. Replaced skipNodeModulesBundle with declaring all @turf/ libraries as external so tsup wouldn't bundle a package's dependencies in with it. escheck moves from lint:* to it's own thing so we can lint during dev without needing JS build to escheck. It still gets run automatically before a release takes place. Dropped a clumsy sweepline-intersections re-re-export shim that was tied to previous moduleResolution: node16 Added tsconfig.types.json for types.ts entry points to relevant projects.
….json package files.
… and tsup.config.ts package files.
| "erasableSyntaxOnly": true, | ||
| "composite": true, | ||
| "allowImportingTsExtensions": true, | ||
| "outDir": "dist", |
There was a problem hiding this comment.
I'm kind of surprised this doesn't need to be "${configDir}/dist", but it seems to work 🤷
| "escheck": "pnpm run /escheck:.*/", | ||
| "escheck:cjs": "es-check es8 packages/*/dist/cjs/index.cjs packages/turf/turf.min.js", | ||
| "escheck:esm": "es-check --module es8 packages/*/dist/esm/index.js", | ||
| "escheck:web": "es-check es5 packages/turf/turf.min.js", |
There was a problem hiding this comment.
lmao we're still shipping es5 turf.min.js. That should definitely get bumped way forward.
#2989
| "release": "pnpm run test:release && pnpm publish -r", | ||
| "rollup": "lerna run --scope @turf/turf rollup", | ||
| "test": "pnpx lerna run test", | ||
| "test:release": "pnpm run lint && pnpm run escheck && lerna run --scope @turf/turf last-checks" |
There was a problem hiding this comment.
I would be in favor of just keeping this all one test script, instead of breaking it out differently for releases.
We can probably entirely get rid of the escheck step once we swap TypeScript to targeting esnext.
| } from "geojson"; | ||
| import type { Intersection } from "sweepline-intersections"; | ||
| import { sweeplineIntersections as findIntersections } from "./lib/sweepline-intersections-export.js"; | ||
| import findIntersections from "sweepline-intersections"; |
There was a problem hiding this comment.
I didn't realize this shim stuff was still around. 🧹
| splitting: true, | ||
| external: [ | ||
| /^@turf\//, // Externalize all @turf workspace packages | ||
| ], |
There was a problem hiding this comment.
Is this required to basically reverse the paths changes when we're doing the tsup step?
They should be already excluded by default if I'm reading the docs right.
| "include": [ | ||
| "index.ts", | ||
| "lib/**/*.ts" | ||
| ], |
There was a problem hiding this comment.
I think this can just be moved into tsconfig.shared.json like so:
"include": [
"${configDir}/index.ts",
"${configDir}/lib/**/*.ts"
]
There was a problem hiding this comment.
Will give it a try.
| { | ||
| "extends": "./tsconfig.json", | ||
| "compilerOptions": { | ||
| "composite": false |
There was a problem hiding this comment.
I'm a little surprised you can't use the default tsconfig.json for the build step as well.
It might be a little extra work if you do the build step in every package as it will re-check any shared dependencies have already been built, but I think that a build of @turf/turf should just recursively build all packages for free if we're doing it in CI.
|
Putting this on ice until we convert remaining JS in turf packages. Otherwise adding in complexity only to remove it again in the near future. |
2 participants
Changed multiple build and test processes to enable dev and testing (as maintainers) without having to build to ESM+CJS Javascript first.
In short:
buildpre-requisite from thetesttarget - test TS directly, including dependent packagesVastly reduces the amount of uncessary JS we need to compile to test TS code. Developer workflow can be as quick as:
Shouldn't result in the generation of a single line of Javascript!
To eventually generate JS for publishing there are new top level targets
prereleaseandrelease. Part of this PR review can be deciding what acitarget needs to do as well.File by file checks on the contents of dist/ indicate no changes to generated ESM or CJS other than a handful of lines removed because we were able to remove an old node16 shim. Debugging map file symbols do change though that's somewhat expected.
Scaffolding changes committed first, and then the generated files separately to help with reviewing.
Please provide the following when creating a PR:
contributorsfield ofpackage.json- you've earned it! 👏