Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Enable generation of ESM so that it can be used with native imports #123

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions .eslintrc.js

This file was deleted.

23 changes: 23 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't spot a difference to the previous config. What's the idea behind this change?

Copy link
Author

@ElMassimo ElMassimo Mar 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned it in the description:

Moved .eslintrc.js to .eslintrc.json because some the older ESLint versions tsdx depends on would fail to require it now that the closest package.json is of "type": "module".

"plugins":[
"es5"
],
"rules":{
"es5/no-for-of":"error",
"es5/no-generators":"error",
"es5/no-typeof-symbol":"error",
"es5/no-es6-methods":"error",
"es5/no-es6-static-methods":[
"error",
{
"exceptMethods":[
"Object.assign"
]
}
],
"no-restricted-syntax":[
"error",
"ForInStatement"
]
}
}
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
run: yarn build

- name: Run benchmark
run: node benchmark.js | tee output.txt
run: yarn benchmark
env:
NODE_ENV: production

Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
node_modules
dist
coverage
output.txt
yarn-error.log
benchmark.mjs
4 changes: 2 additions & 2 deletions benchmark.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const Benchmark = require('benchmark');
const SuperJSON = require('./dist/').default;
import Benchmark from 'benchmark';
import SuperJSON from './dist/index.js';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason to use require here was that it enabled running node benchmark.js, which is quite helpful in dev. Is this change neccessary?

Copy link
Author

@ElMassimo ElMassimo Mar 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running node benchmark.js fails in the latest node versions after adding "type": "module" if using require.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a script to ensure the benchmark can be run in Node 10 as well, by enabling modules with the experimental flag. You can now run yarn benchmark.


const instances = {
'toy example': {
Expand Down
15 changes: 10 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{
"version": "1.7.2",
"license": "MIT",
"main": "dist/index.js",
"main": "dist/index.cjs",
"module": "dist/index.js",
"typings": "dist/index.d.ts",
"type": "module",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the addition of ESM exports!

"files": [
"dist",
"src"
Expand All @@ -11,11 +13,13 @@
"node": ">=10"
},
"scripts": {
"build": "tsc",
"clean": "rm -rf ./dist",
"benchmark": "./scripts/benchmark",
"build": "./scripts/build",
"test": "tsdx test --notify --verbose",
"lint": "tsdx lint",
"prepack": "tsc",
"prepare": "husky install"
"prepare": "husky install",
"prepublishOnly": "npm run build"
},
"importSort": {
".ts": {
Expand Down Expand Up @@ -46,7 +50,6 @@
"url": "https://twitter.com/flybayer"
}
],
"module": "dist/superjson.esm.js",
"repository": {
"type": "git",
"url": "https://github.com/blitz-js/superjson"
Expand All @@ -60,12 +63,14 @@
"husky": "^5.0.9",
"mongodb": "^3.6.4",
"tsdx": "^0.14.1",
"tsup": "^4.0.0",
"typescript": "^4.1.5"
},
"dependencies": {
"debug": "^4.3.1"
},
"resolutions": {
"**/eslint": "^7.17.0",
"**/@typescript-eslint/eslint-plugin": "^4.11.1",
"**/@typescript-eslint/parser": "^4.11.1",
"**/jest": "^26.6.3",
Expand Down
12 changes: 12 additions & 0 deletions scripts/benchmark
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash
currentver="$(node --version)"
requiredver="v12.0.0"

if [ "$(printf '%s\n' "$requiredver" "$currentver" | sort -V | head -n1)" = "$requiredver" ]; then
echo "Benchmarking ESM"
node benchmark.js | tee output.txt
else
echo "Benchmarking CJS"
cp benchmark.js benchmark.mjs
node --experimental-modules benchmark.mjs | tee output.txt
fi
11 changes: 11 additions & 0 deletions scripts/build
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash
currentver="$(node --version)"
requiredver="v12.0.0"

if [ "$(printf '%s\n' "$requiredver" "$currentver" | sort -V | head -n1)" = "$requiredver" ]; then
echo "Building with tsup"
node --experimental-worker ./node_modules/.bin/tsup src/index.ts --dts --sourcemap --target es5 --format cjs,esm
else
echo "Fallback to tsc"
./node_modules/.bin/tsc && cp dist/index.js dist/index.cjs
fi
30 changes: 19 additions & 11 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,21 +1,29 @@
{
"include": ["src"],
"exclude": ["**/*.spec.ts", "**/*.test.ts"],
"compilerOptions": {
"target": "es5",
"module": "CommonJS",
"lib": ["esnext"],
"importHelpers": false,
"declaration": true,
"sourceMap": true,
"rootDir": "./src",
"outDir": "./dist",
"strict": true,
"declaration": true,
"sourceMap": true,
"importHelpers": false,
"downlevelIteration": true,
"esModuleInterop": true,
"moduleResolution": "Node",
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,
"moduleResolution": "node",
"esModuleInterop": true,
"downlevelIteration": true
}
"resolveJsonModule": true,
"skipLibCheck": true,
"strict": true,
"strictNullChecks": true
},
"exclude": [
"**/dist",
"**/node_modules",
"**/test",
"**/*.spec.ts",
"**/*.test.ts"
]
}
Loading