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

build: use pkgroll #28

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

privatenumber
Copy link
Collaborator

Problem

  • Using TypeScript to build is slow and requires a complicated setup
  • Benchmark:
$ hyperfine 'npm run build'
Benchmark 1: npm run build
  Time (mean ± σ):      7.043 s ±  0.184 s    [User: 13.169 s, System: 0.690 s]
  Range (min … max):    6.815 s …  7.372 s    10 runs

Changes

  • Use pkgroll for a simpler & faster build step

  • Restructure dist directory to be simpler

    • This could be a breaking change for users that are directly importing a specific path
  • Remove tools/post-build.js as pkgroll automatically outputs .mjs file

  • I decided not to output sourcemaps because the distribution file is not very different from the source file. Also, because Node.js is the primary use-case, which can't load source-maps out of the box. I can add it back if you'd like.

  • Removed unused tsconfig files

  • Refactored tsconfig.json to remove irrelevant properties and speed it up

  • Benchmark:

$ hyperfine 'npm run build'
Benchmark 1: npm run build
  Time (mean ± σ):      1.928 s ±  0.073 s    [User: 2.906 s, System: 0.295 s]
  Range (min … max):    1.873 s …  2.075 s    10 runs

@changeset-bot
Copy link

changeset-bot bot commented Feb 4, 2023

⚠️ No Changeset found

Latest commit: 4698137

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

"scripts": {
"test": "mocha -r @esbuild-kit/cjs-loader --extension ts,js src/*.test.ts",
"build": "rimraf dist/ && tsc && tsc -p tsconfig.module.json && tsc -p tsconfig.esm.json && node tools/post-build.js",
"type-check": "tsc --noEmit",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we're no longer using tsc to build, this is just a convenience to use tsc only for type-checking during development. Can be removed if you rely on the IDE for type-checking.

For building, npm run build still type-checks because it uses TypeScript to generate the .d.ts file.

@@ -2,11 +2,7 @@
"compilerOptions": {
"strict": true,
"target": "ES2019",
"module": "CommonJS",
"moduleResolution": "node",
"outDir": "dist/cjs/",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No longer used. Building is now handled by pkgroll.

@@ -2,11 +2,7 @@
"compilerOptions": {
"strict": true,
"target": "ES2019",
"module": "CommonJS",
"moduleResolution": "node",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

kolorist doesn't import anything so not really relevant.

"outDir": "dist/cjs/",
"sourceMap": true,
"allowJs": true
"skipLibCheck": true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Faster type-checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant