Skip to content

Commit

Permalink
Fix ESM/CJS compatibility
Browse files Browse the repository at this point in the history
Superstruct cannot be imported in a TypeScript project that uses a
`moduleResolution` of `node16` or `nodenext`. Any attempt to do so will result
in type errors such as the following:

```
node_modules/superstruct/dist/index.d.ts:1:15 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

1 export * from './error';
                ~~~~~~~~~

node_modules/superstruct/dist/index.d.ts:2:15 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

2 export * from './struct';
                ~~~~~~~~~~

node_modules/superstruct/dist/index.d.ts:3:15 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

3 export * from './structs/coercions';
                ~~~~~~~~~~~~~~~~~~~~~

...
```

This is happening because although Superstruct is defined as an ESM library —
the `module` field in `package.json` is set to `true` — its published TypeScript
declaration files aren't ESM-compatible. This is due to the fact that imports
are missing extensions, which is a requirement for ESM.

At the same time, although Rollup is supposedly configured to emit both ESM- and
CJS-compatible files, TypeScript does not know how to use the CJS variants. The
`exports` field is the way to instruct TypeScript on how to do this, but
`package.json` is missing this field. This makes it impossible to use
Superstruct in a non-ESM library.

This commit fixes both of these issues by making the following changes:

- All imports in TypeScript files now use an extension of `.js`. This may seem
  like an odd choice, as there are no JavaScript files in the `src/` directory,
  but TypeScript doesn't resolve a module by trying to find a file with the
  given extension; it tries to find a declaration file that maps to the module
  path, based on a set of rules. Therefore, the file extension is really just a
  formality. In the end, Rollup is consolidating implementation files into one,
  so all of the imports will go away.
- Type declaration files have been split into ESM and CJS variants. Since Rollup
  generates ESM variants with an `.mjs` extension and CJS variants with a `.cjs`
  extension, imports in declaration files also use either `.mjs` or `.cjs`
  depending on the variant.
- `package.json` now has an `exports` field which instructs TypeScript how to
  resolve modules from either an ESM or CJS perspective.

In addition, the tests have been updated to use Vitest instead of Babel so that
they can read directly from the TypeScript configuration instead of having to
use an explicit transform step.

In conjunction with this change, the minimum Node version has been bumped to
16.x and the GitHub workflows have been updated to stop testing on Node 14.x.
14.x has reached EOL, the version of Vitest we're using isn't compatible with
this version, and a `target` of `esnext` is being used in the TypeScript config,
which require a JavaScript runtime that supports newer ES features such as
`??=`, so the minimum Node version is effectively 16.x anyway.

This commit is derived from the following PR on the Superstruct repo:
<ianstormtaylor#1211>

For more information on how TypeScript resolves modules, the "Modules Reference"
section of the TypeScript handbook is quite helpful, and in particular these
sections:

- <https://www.typescriptlang.org/docs/handbook/modules/theory.html#the-role-of-declaration-files>
- <https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext>
  • Loading branch information
mcmire committed Feb 9, 2024
1 parent fd2b6e7 commit 4e10994
Show file tree
Hide file tree
Showing 65 changed files with 140 additions and 113 deletions.
15 changes: 0 additions & 15 deletions .babelrc

This file was deleted.

8 changes: 7 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@
"node": true
},
"settings": {
"import/extensions": [".js", ".ts"]
"import/extensions": [".js", ".ts"],
"import/resolver": {
"typescript": {
"project": ["./tsconfig.json"]
}
}
},
"ignorePatterns": ["/dist/**"],
"rules": {
"@typescript-eslint/no-unused-vars": [
"error",
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [14.x, 16.x, 18.x]
node-version: [16.x, 18.x]
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v2
Expand Down
39 changes: 24 additions & 15 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,21 @@
"license": "MIT",
"repository": "git://github.com/MetaMask/superstruct.git",
"type": "module",
"exports": {
".": {
"import": {
"default": "./dist/index.mjs",
"types": "./dist/index.d.mts"
},
"require": {
"default": "./dist/index.cjs",
"types": "./dist/index.d.cts"
}
}
},
"main": "./dist/index.cjs",
"module": "./dist/index.mjs",
"types": "./dist/index.d.ts",
"types": "./dist/index.d.cts",
"sideEffects": false,
"files": [
"dist"
Expand All @@ -16,39 +28,36 @@
"registry": "https://registry.npmjs.org"
},
"engines": {
"node": ">=14.0.0"
"node": ">=16.0.0"
},
"devDependencies": {
"@babel/cli": "^7.6.3",
"@babel/core": "^7.6.3",
"@babel/plugin-transform-modules-commonjs": "^7.12.1",
"@babel/preset-env": "^7.20.2",
"@babel/preset-typescript": "^7.6.0",
"@babel/register": "^7.6.2",
"@rollup/plugin-typescript": "^9.0.2",
"@types/expect": "^24.3.0",
"@types/lodash": "^4.14.144",
"@types/lodash-es": "^4.17.12",
"@types/mocha": "^10.0.0",
"@types/node": "^18.7.14",
"@typescript-eslint/eslint-plugin": "^5.43.0",
"@typescript-eslint/parser": "^5.43.0",
"babel-eslint": "^10.0.3",
"eslint": "^7.14.0",
"eslint-config-prettier": "^7.2.0",
"eslint-plugin-import": "^2.22.1",
"eslint-import-resolver-typescript": "^3.6.1",
"eslint-plugin-import": "^2.29.1",
"eslint-plugin-prettier": "^4.0.0",
"is-email": "^1.0.0",
"is-url": "^1.2.4",
"is-uuid": "^1.0.2",
"lodash": "^4.17.15",
"jest": "^29.7.0",
"lodash-es": "^4.17.21",
"mocha": "^10.0.0",
"np": "^7.6.2",
"prettier": "^2.0.5",
"rollup": "^3.3.0",
"typescript": "^4.8.3"
"typescript": "^4.8.3",
"vitest": "^1.2.2"
},
"scripts": {
"build": "rm -rf ./{dist} && rollup --config ./rollup.config.js",
"build": "rm -rf ./dist && rollup --config ./rollup.config.js && scripts/split-tsd-files.sh",
"clean": "rm -rf ./{dist,node_modules}",
"fix": "npm run fix:eslint && npm run fix:prettier",
"fix:eslint": "npm run lint:eslint --fix",
Expand All @@ -57,8 +66,8 @@
"lint:eslint": "eslint '{src,test}/*.{js,ts}'",
"lint:prettier": "prettier --list-different '**/*.{js,json,ts}'",
"release": "npm run build && npm run lint && np",
"test": "npm run build && npm run test:types && npm run test:mocha",
"test:mocha": "mocha --require ./test/register.cjs --require source-map-support/register ./test/index.ts",
"test": "npm run build && npm run test:types && npm run test:vitest",
"test:vitest": "vitest run",
"test:types": "tsc --noEmit && tsc --project ./test/tsconfig.json --noEmit",
"watch": "npm run build -- --watch"
},
Expand Down
22 changes: 22 additions & 0 deletions scripts/split-tsd-files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash

set -euo pipefail

for file in dist/*.d.ts dist/structs/*.d.ts; do
mv $file "${file%.d.ts}.d.cts"
cp "${file%.d.ts}.d.cts" "${file%.d.ts}.d.mts"
done

for file in dist/*.d.cts dist/structs/*.d.cts; do
echo "File: $file"
sed -e 's/\.d\.ts/.d.cts/' -i.bak "$file"
sed -e 's/\.js/.cjs/' -i.bak "$file"
done

for file in dist/*.d.mts dist/structs/*.d.mts; do
echo "File: $file"
sed -e 's/\.d\.ts/\.d\.mts/' -i.bak "$file"
sed -e 's/\.js/.mjs/' -i.bak "$file"
done

find dist -name '*.bak' | xargs rm
12 changes: 6 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export * from './error'
export * from './struct'
export * from './structs/coercions'
export * from './structs/refinements'
export * from './structs/types'
export * from './structs/utilities'
export * from './error.js'
export * from './struct.js'
export * from './structs/coercions.js'
export * from './structs/refinements.js'
export * from './structs/types.js'
export * from './structs/utilities.js'
4 changes: 2 additions & 2 deletions src/struct.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { toFailures, shiftIterator, StructSchema, run } from './utils'
import { StructError, Failure } from './error'
import { toFailures, shiftIterator, StructSchema, run } from './utils.js'
import { StructError, Failure } from './error.js'

/**
* `Struct` objects encapsulate the validation logic for a specific type of
Expand Down
6 changes: 3 additions & 3 deletions src/structs/coercions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Struct, is, Coercer } from '../struct'
import { isPlainObject } from '../utils'
import { string, unknown } from './types'
import { Struct, is, Coercer } from '../struct.js'
import { isPlainObject } from '../utils.js'
import { string, unknown } from './types.js'

/**
* Augment a `Struct` to add an additional coercion step to its input.
Expand Down
4 changes: 2 additions & 2 deletions src/structs/refinements.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Struct, Refiner } from '../struct'
import { toFailures } from '../utils'
import { Struct, Refiner } from '../struct.js'
import { toFailures } from '../utils.js'

/**
* Ensure that a string, array, map, or set is empty.
Expand Down
6 changes: 3 additions & 3 deletions src/structs/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Infer, Struct } from '../struct'
import { define } from './utilities'
import { Infer, Struct } from '../struct.js'
import { define } from './utilities.js'
import {
ObjectSchema,
ObjectType,
Expand All @@ -9,7 +9,7 @@ import {
AnyStruct,
InferStructTuple,
UnionToIntersection,
} from '../utils'
} from '../utils.js'

/**
* Ensure that any value passes validation.
Expand Down
11 changes: 8 additions & 3 deletions src/structs/utilities.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { Context, Struct, Validator } from '../struct'
import { Assign, ObjectSchema, ObjectType, PartialObjectSchema } from '../utils'
import { object, optional, type } from './types'
import { Context, Struct, Validator } from '../struct.js'
import {
Assign,
ObjectSchema,
ObjectType,
PartialObjectSchema,
} from '../utils.js'
import { object, optional, type } from './types.js'

/**
* Create a new struct that combines the properties properties from multiple
Expand Down
4 changes: 2 additions & 2 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Struct, Infer, Result, Context, Describe } from './struct'
import { Failure } from './error'
import { Struct, Infer, Result, Context, Describe } from './struct.js'
import { Failure } from './error.js'

/**
* Check if a value is an iterator.
Expand Down
1 change: 1 addition & 0 deletions test/api/assert.ts → test/api/assert.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { throws, doesNotThrow } from 'assert'
import { assert, string, StructError } from '../../src'
import { describe, it } from 'vitest'

describe('assert', () => {
it('valid as helper', () => {
Expand Down
1 change: 1 addition & 0 deletions test/api/create.ts → test/api/create.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { describe, it } from 'vitest'
import { strictEqual, deepEqual, deepStrictEqual, throws } from 'assert'
import {
type,
Expand Down
1 change: 1 addition & 0 deletions test/api/is.ts → test/api/is.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { describe, it } from 'vitest'
import { strictEqual } from 'assert'
import { is, string } from '../../src'

Expand Down
1 change: 1 addition & 0 deletions test/api/mask.ts → test/api/mask.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { describe, it } from 'vitest'
import { deepStrictEqual, throws } from 'assert'
import {
mask,
Expand Down
1 change: 1 addition & 0 deletions test/api/validate.ts → test/api/validate.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { describe, it } from 'vitest'
import { deepStrictEqual, strictEqual } from 'assert'
import {
validate,
Expand Down
16 changes: 5 additions & 11 deletions test/index.ts → test/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert, { CallTracker } from 'assert'
import fs from 'fs'
import { pick } from 'lodash'
import { pick } from 'lodash-es'
import { basename, extname, resolve } from 'path'
import {
any,
Expand All @@ -12,15 +12,9 @@ import {
StructError,
} from '../src'

describe('superstruct', () => {
describe('api', () => {
require('./api/assert')
require('./api/create')
require('./api/is')
require('./api/mask')
require('./api/validate')
})
import { describe, it } from 'vitest'

describe('superstruct', () => {
describe('validation', () => {
const kindsDir = resolve(__dirname, 'validation')
const kinds = fs
Expand All @@ -29,15 +23,15 @@ describe('superstruct', () => {
.map((t) => basename(t, extname(t)))

for (const kind of kinds) {
describe(kind, () => {
describe(kind, async () => {
const testsDir = resolve(kindsDir, kind)
const tests = fs
.readdirSync(testsDir)
.filter((t) => t[0] !== '.')
.map((t) => basename(t, extname(t)))

for (const name of tests) {
const module = require(resolve(testsDir, name))
const module = await import(resolve(testsDir, name))
const { Struct, data, create, only, skip, output, failures } = module
const run = only ? it.only : skip ? it.skip : it
run(name, () => {
Expand Down
3 changes: 0 additions & 3 deletions test/register.cjs

This file was deleted.

6 changes: 5 additions & 1 deletion test/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{
"extends": "../tsconfig.json",
"include": ["./**/*.ts"]
"include": ["./**/*.ts"],
"compilerOptions": {
"lib": ["esnext"],
"skipLibCheck": true
}
}
2 changes: 1 addition & 1 deletion test/typings/any.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, any } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<any>((x) => {
assert(x, any())
Expand Down
2 changes: 1 addition & 1 deletion test/typings/array.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, array, number } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<Array<unknown>>((x) => {
assert(x, array())
Expand Down
2 changes: 1 addition & 1 deletion test/typings/assign.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, assign, object, number, string } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<{
a: number
Expand Down
2 changes: 1 addition & 1 deletion test/typings/bigint.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, bigint } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<bigint>((x) => {
assert(x, bigint())
Expand Down
2 changes: 1 addition & 1 deletion test/typings/boolean.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, boolean } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<boolean>((x) => {
assert(x, boolean())
Expand Down
2 changes: 1 addition & 1 deletion test/typings/coerce.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, coerce, string, number } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<number>((x) => {
assert(
Expand Down
2 changes: 1 addition & 1 deletion test/typings/date.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, date } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<Date>((x) => {
assert(x, date())
Expand Down
2 changes: 1 addition & 1 deletion test/typings/defaulted.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, defaulted, string } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<string>((x) => {
assert(x, defaulted(string(), 'Untitled'))
Expand Down
2 changes: 1 addition & 1 deletion test/typings/deprecated.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, object, deprecated, any, Context } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<unknown>((x) => {
const log = (value: unknown, ctx: Context) => {}
Expand Down
2 changes: 1 addition & 1 deletion test/typings/describe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
min,
pattern,
} from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<Describe<any>>((x) => {
return any()
Expand Down
2 changes: 1 addition & 1 deletion test/typings/dynamic.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, dynamic, string } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<string>((x) => {
assert(
Expand Down
2 changes: 1 addition & 1 deletion test/typings/empty.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert, empty, string, array, map, set } from '../../src'
import { test } from '..'
import { test } from '../index.test'

test<string>((x) => {
assert(x, empty(string()))
Expand Down
Loading

0 comments on commit 4e10994

Please sign in to comment.