ReviewOS

also looking at this

craft-native/craft

fix(sdk): ship declarations that resolve under node16/nodenext

#42
Merged glennmichael123 wants to merge fix/node16-type-resolution into main
43 files +196 -137
11 days ago

Found while auditing #33. Not one of that issue's claims — a bigger one behind them.

The defect

Install craft-native into any TypeScript project using moduleResolution: node16 or nodenext — the modern Node default — and you get:

node_modules/craft-native/dist/index.d.ts(5,47): error TS2834: Relative import paths need
  explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'
consumer.ts(1,10): error TS2305: Module '"craft-native/mobile"' has no exported member 'camera'

dist/index.d.ts shipped 15 extensionless re-exports and dist/mobile.d.ts one, so both the root entry and the browser-facing craft-native/mobile subpath resolved to zero types. The exports map is correct and the JS is fine; the declarations are simply unresolvable.

Nothing in this repo noticed because the repo builds itself with moduleResolution: bundler, where extensionless specifiers are legal. tsc enforces the rule only on the consumer side, and there was no consumer-side test — so emit stayed happy and every downstream project got nothing.

The fix

Relative specifiers in src now carry .js, and /index.js for directory imports. Under bundler resolution tsc still maps ./api/mobile.js back to ./api/mobile.ts, so the build is unchanged — the emitted declarations just become resolvable. 116 specifiers across 35 files, mechanical.

ts-maps had the identical defect and gets the identical fix.

The guard

verify-packages.ts now walks every shipped .d.ts/.d.cts/.d.mts and fails on any relative specifier without a file extension. I wrote it before finishing the fix, and it immediately caught packages/ts-maps/dist/MapView.d.ts importing "./types" — the typescript package alone would have been a half fix. That is the check that was missing, not just the extensions.

Verification

Not "it compiles" — an actual consumer:

  1. npm pack the real tarball
  2. install it into a scratch project with "module": "nodenext", "moduleResolution": "nodenext" and @types/node
  3. typecheck import { createApp } from 'craft-native' and import { camera, haptics } from 'craft-native/mobile'
entrybeforeafter
craft-nativeTS2834 ×15clean
craft-native/mobileTS2834 + TS2305clean

Plus: root bun run typecheck (typescript, create-craft, ts-maps) clean; bun test 468 pass in the SDK and 27 in ts-maps; bun scripts/verify-packages.ts → 36 checks; pickier 0 errors.

Relationship to #33

#33's own three claims are already fixed in the tree — I'll write that up separately with the evidence. This is the defect that was underneath them, and it is the one that actually stops @stacksjs/mobile from consuming the package.

43 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.