Skip to content

Audit package.json and tsconfig.json #522

@TheMrCam

Description

@TheMrCam

Overview

We should go through the package.json and tsconfig.json/tsconfig.build.json files and clean up what we can. This issue is a public brain dump of my thoughts, in order to attract others' thoughts 🙂.

package.json

  • Remove build:types script
  • Remove prelint script
    • Remove need to build:module before test
  • Fix docs on Windows
  • Add src to "files"
  • Update engines.node

tsconfig.json

Details

package.json

scripts

I'm mostly focused on the scripts and pre* automations. Some of it seems rather inefficient.

geotiff.js/package.json

Lines 81 to 101 in ae88c5e

"scripts": {
"prebuild": "npm run build:clean",
"build": "npm run build:module && run-p build:browser build:node",
"build:clean": "shx rm -rf dist-node/ dist-browser/ dist-module/",
"build:module": "tsc && rollup -c rollup.worker.config.js",
"build:browser": "rollup -c rollup.config.js",
"build:node": "tsc --project tsconfig.build.json && shx echo \"{\\\"type\\\":\\\"commonjs\\\"}\" > dist-node/package.json",
"build:types": "tsc --outdir dist-module/",
"watch:module": "chokidar \"src/*.js\" -c \"npm run build:module\"",
"watch:browser": "chokidar \"dist-module/*.js\" -c \"npm run build:browser\"",
"predev": "npm run build",
"dev": "run-p watch:module watch:browser dev:serve",
"dev:serve": "serve --listen 8090",
"docs": "rm -rf docs/; typedoc",
"prelint": "npm run build:module",
"lint": "eslint src test .eslintrc.cjs",
"lint:fix": "npm run lint -- --fix",
"prepare": "npm run build",
"pretest": "npm run lint && tsc --noEmit",
"test": "mocha --full-trace test/*.spec.js"
},

  • build: details below.
  • build:types: no longer used, tsc now generates types by default (i.e. in build:module).
  • docs: still has the Windows bug detailed in Actually fix windows build #190 (; instead of &).
  • prelint: I don't think we need to build dist-module/ in order to lint src/ and test/.
    • At the moment, we do need to build dist-module/ before npm run test, since that's what test/geotiff.spec.js imports.
      • I believe this is undesirable.
  • pretest: tsc --noEmit feels clunky here; I feel like that part belongs either in test or lint.
    • I convinced myself that I don't mind pretest, it's mostly just prelint that's problematic.
  • I don't use watch* or dev*.

build

The build script (and output) feels clunky and confusing. The process after typing npm run build:

  1. "prebuild": "npm run build:clean"
    • This is fine. I feel like build tools can usually do this now, but "if it ain't broke, don't fix it."
  2. "build": "npm run build:module && run-p build:browser build:node"
    1. "build:module": "tsc && rollup -c rollup.worker.config.js"
      • Generate dist-module/, including .d.ts types & an inlined src/worker/decode.js worker
    2. "build:browser": "rollup -c rollup.config.js"
      • Cool, building the UMD bundle... from dist-module/? Because of dist-module/worker/create.js from rollup?
    3. "build:node": "tsc --project tsconfig.build.json && shx echo \"{\\\"type\\\":\\\"commonjs\\\"}\" > dist-node/package.json"
      • Okay... building dist-node/ CJS files from dist-module/, then creating dist-node/package.json to notate the CJS type. Again from dist-module/ because of rollup's dist-module/worker/create.js, I guess?
      • Importantly, this step disregards dist-module/*.d.ts files and regenerates new ones from the JSDoc comments in dist-module/*.js files. This results in a loss of some type information, especially from originally-TypeScript files. However, this doesn't particularly matter, because the "types" are always retrieved from dist-module/. So we're ultimately generating a lot of files for no reason.

Writing this out has helped me understand the process a bit, but it still feels like it can be improved. My experience with Vite library mode is only tangentially helpful for geotiff.js—the "help wanted" label is mostly for opinions on building. We can even make a new issue for discussing the build step if preferred.

files

geotiff.js/package.json

Lines 28 to 32 in ae88c5e

"files": [
"dist-module",
"dist-node",
"dist-browser"
],

I believe adding src is all that's necessary to enable correct "Go to Definition" jumps, regardless of if the definition is in a .ts or .js file. See #487 (comment) for an example.

engines

geotiff.js/package.json

Lines 33 to 35 in ae88c5e

"engines": {
"node": ">=10.19"
},

Do we still support NodeJS >= 10.19? Do we want to?
I bumped to >= 14.0.0 in #494.
Now I'm leaning towards >= 22.18 for TypeScript + erasableSyntaxOnly, but we don't need to limit it that much yet (especially while still building dist-node/).

exports

geotiff.js/package.json

Lines 20 to 26 in ae88c5e

"exports": {
".": {
"import": "./dist-module/geotiff.js",
"require": "./dist-node/geotiff.js",
"browser": "./dist-browser/geotiff.js"
}
},

We might be able to do something like point "node" "exports" at src/—although only for NodeJS >= 22.18 (see below), and I'm not sure if conditional exports based on the version are possible.

tsconfig.json

Related to #493.

"compilerOptions": {
"outDir": "dist-module",
"target": "ES2020",
"lib": ["ES2020", "DOM", "ES2021.WeakRef"],
"module": "ES2020",
"allowJs": true,
"checkJs": true,
"declaration": true,
"declarationMap": true,
"sourceMap": true,
"importHelpers": false,
"noImplicitAny": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"strict": true,
"strictNullChecks": true,
"skipLibCheck": true,
"moduleResolution": "bundler",
"esModuleInterop": false,
"inlineSources": false,
"downlevelIteration": true,
},

erasableSyntaxOnly

Based on the discussions in #487, I strongly feel that we should enable erasableSyntaxOnly to preserve native execution in NodeJS (>=22.18)

Others

We'd need to go through the other options to figure out what we actually need. For example, "strict": true is already enabling a few of the other type checking options (noImplicitAny, strictNullChecks). Maybe the redundancy is desirable since it's more explicit, but I'd rather trim out as much as we can.

A non-exhaustive list of potential changes:

  • target & lib: are we happy with "ES2020" and ["ES2020", "DOM", "ES2021.WeakRef"], and is it the minimum we can/want to go?
  • esModuleInterop: I'm mostly just curious why this is false for tsconfig.json, then true for tsconfig.build.json. Does it need to be that way, or what benefit is that providing?
  • downlevelIteration: I'm not sure if we need this but it's probably not hurting.
tsconfig.build.json

"compilerOptions": {
"target": "ES2020",
"module": "commonjs",
"outDir": "./dist-node",
"lib": ["ES2020"],
"allowJs": true,
"declaration": true,
"declarationMap": true,
"sourceMap": true,
"importHelpers": false,
"skipLibCheck": true,
"strict": false,
"strictNullChecks": true,
"moduleResolution": "node",
"esModuleInterop": true,
"inlineSources": false,
"downlevelIteration": true
},

Provided for comparison.

The only real differences are lib, module, moduleResolution, esModuleInterop, and strict. We could probably get by with just tsconfig.json and CLI options for build:node.

Metadata

Metadata

Assignees

Labels

buildRelated to building and packaginghelp wanted

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions