chore: build project to emit cjs and esm using rollup #93

amoshydra · 2024-04-10T08:26:59Z

Edit 1: Updated base on commit 759f5b7

Summary

⚠ BREAKING CHANGE: distribution layout has been significantly changed

Support both cjs and mjs output using rollup.

The package.json now includes the modern exports field and the legacy main, modules and typings fields to ensure compatibiilty with older Node runtime / bundler

Related to

Related to #61

Motivation

To resolve #61

How is this tested

Fixtures for the different environments

✔ retested for 759f5b7 (code identical with latest @ d4c8909)

I have created a repository hosting the 4 test environments below here: https://github.com/amoshydra/repro-jessealama-decimal128/tree/ref-759f5b791

browser importing mjs
nodejs [type="module"] importing mjs
nodejs [type="commonjs"] importing cjs
bundler (vite --> rollup / esbuild) importing mjs

browser mjs	nodejs cjs	nodejs mjs	vite mjs

Layout change in npm package

This layout change does not really affect the consumer using a bundler, TypeScript or NodeJS as they will continue to import the package via one of the following ways:

// esm
import { Decimal128 } from "decimal128";
// cjs
const { Decimal128 } = require("decimal128");

Before

Files are exported from src

src
├── common.d.mts
├── common.mjs
├── common.mts
├── decimal128.d.mts
├── decimal128.mjs
├── decimal128.mts
├── rational.d.mts
├── rational.mjs
└── rational.mts

After

Files are exported from dist

dist
├── cjs
│   ├── common.cjs
│   ├── common.cjs.map
│   ├── common.d.mts
│   ├── decimal128.cjs
│   ├── decimal128.cjs.map
│   ├── decimal128.d.mts
│   ├── rational.cjs
│   ├── rational.cjs.map
│   └── rational.d.mts
└── esm
    ├── common.d.mts
    ├── common.mjs
    ├── common.mjs.map
    ├── decimal128.d.mts
    ├── decimal128.mjs
    ├── decimal128.mjs.map
    ├── rational.d.mts
    ├── rational.mjs
    └── rational.mjs.map

amoshydra · 2024-04-10T10:28:36Z

rollup.config.mjs

+    typescript({
+      tsconfig: "tsconfig.build.json",
+      outDir: "dist",
+      sourceMap: false,


Note to reviewer

Source map is still generated by using rollup's output.sourcemap: true: https://github.com/jessealama/decimal128/pull/93/files/7feb5cd9d46d29115d1af57986e6a00ad23daeed#diff-47407fecafdf5f5cd55403c3de457833ddf9b6fab45253c04e1dc4c7cb4495b1R8.

This is turned off to prevent conflict with rollup's source map generation
See related stackoverflow post: https://stackoverflow.com/a/63235210

tsconfig.json

amoshydra · 2024-04-10T14:59:19Z

With the new understanding that tests and examples are suppose to run on transpiled code. I have updated the approach to do thing differently.

`output.preserveModules: true`

Because the tests still imports round.mjs and rational.mjs, in the new commit, I have used the output.preserveModules flag to keep the module structure.

Usage of preserveModules require us to output the build files into their own folder. In this PR, I have choose to output esm and cjs to their respective folder accordingly.

Before this option

dist
├── common.d.mts      
├── decimal128.cjs    
├── decimal128.cjs.map
├── decimal128.d.mts  
├── decimal128.mjs
├── decimal128.mjs.map
└── rational.d.mts

After this option

dist
├── cjs
│   ├── common.cjs
│   ├── common.cjs.map
│   ├── common.d.mts
│   ├── decimal128.cjs
│   ├── decimal128.cjs.map
│   ├── decimal128.d.mts
│   ├── rational.cjs
│   ├── rational.cjs.map
│   └── rational.d.mts
└── esm
    ├── common.d.mts
    ├── common.mjs
    ├── common.mjs.map
    ├── decimal128.d.mts
    ├── decimal128.mjs
    ├── decimal128.mjs.map
    ├── rational.d.mts
    ├── rational.mjs
    └── rational.mjs.map

Notice rational and common are kept as its own module

Tests

Tests and coverage can now run like how it previously was

Workflow

Workflow file has been update to include npm run build command:
759f5b7#diff-1790f2f8b7123d50d46235053b54f1cc5996bc7388b803aad4233fd5582e0bad

-              run: npx tsc
+              run: npm run build && npx tsc

jessealama · 2024-04-10T17:57:05Z

Thanks so much for your interest in this!

Could you please rebase on the newly changed main branch?

amoshydra · 2024-04-11T02:02:38Z

sure! rebased onto cf135fd @ #93 (comment)

BREAKING CHANGE: distribution layout has been significantly changed

ryzokuken

Overall it looks good, a few comments inline: also, you might've forgotten to update the .gitignore

ryzokuken · 2024-05-08T12:26:42Z

package.json

        "c8": "^9.1.0",
-        "jest": "^29.5.0",
+        "jest": "^29.7.0",


Was updating jest and ts-jest necessary in this PR?

ryzokuken · 2024-05-08T12:36:24Z

package.json

+            "import": "./dist/esm/decimal128.mjs",
+            "require": "./dist/cjs/decimal128.cjs",


check out nodejs/node#52174, this pattern is not ideal and we should use the alternative mentioned in the thread.

ryzokuken · 2024-05-08T12:40:19Z

tsconfig.build.json

@@ -0,0 +1,4 @@
+{
+    "extends": "./tsconfig.json",
+    "include": ["./src"]


Apologies, but I don't quite get this: which files were included so far? And why not just add the include statement to the original tsconfig?

which files were included so far

In the original tsconfig, I suppose, all files will be included.
I believe that is fine since the original tsconfig is mainly used by VS Code / ide.

why not just add the include statement to the original tsconfig?

Adding includes to the original tsconfig will prevent tsconfig from being applied to files outside of the src folder.

My intention here with tsconfig.buid.json include is to restrict rollup's typescript compiler include scope to files within src folder only

ryzokuken · 2024-05-08T12:42:45Z

package.json

-        "ts-jest": "^29.1.0",
+        "rollup": "^4.14.1",
+        "ts-jest": "^29.1.2",
+        "tslib": "^2.6.2",


I don't see why we need tslib since importHelpers aren't used?

likely a mistake, PR raised by Jesse in #121 no longer has this

jessealama · 2024-11-15T08:45:25Z

closed via #121

amoshydra force-pushed the rollup-cjs-export branch 2 times, most recently from aa01cb5 to 7feb5cd Compare April 10, 2024 09:55

amoshydra changed the title ~~chore: build project to cjs and esm will rollup~~ [Bundler] chore: build project to cjs and esm will rollup Apr 10, 2024

amoshydra commented Apr 10, 2024

View reviewed changes

tsconfig.json Show resolved Hide resolved

amoshydra marked this pull request as ready for review April 10, 2024 10:41

amoshydra changed the title ~~[Bundler] chore: build project to cjs and esm will rollup~~ [Bundler] chore: build project to cjs and esm with rollup Apr 10, 2024

amoshydra force-pushed the rollup-cjs-export branch from 7feb5cd to e822f21 Compare April 10, 2024 10:54

amoshydra mentioned this pull request Apr 10, 2024

Unable to run test due to "Error [ERR_MODULE_NOT_FOUND]: Cannot find module *.mjs" #94

Open

amoshydra changed the title ~~[Bundler] chore: build project to cjs and esm with rollup~~ [Bundler] chore: build project to emit cjs and esm using rollup Apr 10, 2024

amoshydra force-pushed the rollup-cjs-export branch 2 times, most recently from 759f5b7 to be2baa8 Compare April 10, 2024 15:33

This was referenced Apr 10, 2024

[Test] Support loading of mts in test files #95

Closed

[Test] Working test and coverage with ts-jest #96

Closed

amoshydra changed the title ~~[Bundler] chore: build project to emit cjs and esm using rollup~~ chore: build project to emit cjs and esm using rollup Apr 10, 2024

amoshydra force-pushed the rollup-cjs-export branch 2 times, most recently from d4c8909 to 02ded6a Compare April 10, 2024 17:55

amoshydra force-pushed the rollup-cjs-export branch 2 times, most recently from d8623a0 to 4cace43 Compare April 11, 2024 20:36

amoshydra added 2 commits April 16, 2024 09:25

chore: build project to emit cjs and esm using rollup

36cdbcb

BREAKING CHANGE: distribution layout has been significantly changed

chore: restructure dist layout to work with existing tests and examples

69d819d

amoshydra force-pushed the rollup-cjs-export branch from 4cace43 to 69d819d Compare April 16, 2024 01:26

ryzokuken reviewed May 8, 2024

View reviewed changes

ljani mentioned this pull request Oct 15, 2024

decimal128 does not play nicely with tsc #119

Closed

jessealama mentioned this pull request Nov 15, 2024

Emit ESM and CJS, built with rollup #121

Merged

jessealama closed this Nov 15, 2024

amoshydra mentioned this pull request Nov 17, 2024

fix: map exports to dist folder #123

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: build project to emit cjs and esm using rollup #93

chore: build project to emit cjs and esm using rollup #93

amoshydra commented Apr 10, 2024 •

edited

Loading

amoshydra Apr 10, 2024 •

edited

Loading

amoshydra commented Apr 10, 2024 •

edited

Loading

jessealama commented Apr 10, 2024

amoshydra commented Apr 11, 2024

ryzokuken left a comment

ryzokuken May 8, 2024

ryzokuken May 8, 2024

ryzokuken May 8, 2024

amoshydra Nov 17, 2024

ryzokuken May 8, 2024

amoshydra Nov 17, 2024

jessealama commented Nov 15, 2024

		"import": "./dist/esm/decimal128.mjs",
		"require": "./dist/cjs/decimal128.cjs",

chore: build project to emit cjs and esm using rollup #93

chore: build project to emit cjs and esm using rollup #93

Conversation

amoshydra commented Apr 10, 2024 • edited Loading

Summary

Related to

Motivation

How is this tested

Fixtures for the different environments

Layout change in npm package

Before

After

amoshydra Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Note to reviewer

amoshydra commented Apr 10, 2024 • edited Loading

output.preserveModules: true

Before this option

After this option

Tests

Workflow

jessealama commented Apr 10, 2024

amoshydra commented Apr 11, 2024

ryzokuken left a comment

Choose a reason for hiding this comment

ryzokuken May 8, 2024

Choose a reason for hiding this comment

ryzokuken May 8, 2024

Choose a reason for hiding this comment

ryzokuken May 8, 2024

Choose a reason for hiding this comment

amoshydra Nov 17, 2024

Choose a reason for hiding this comment

ryzokuken May 8, 2024

Choose a reason for hiding this comment

amoshydra Nov 17, 2024

Choose a reason for hiding this comment

jessealama commented Nov 15, 2024

amoshydra commented Apr 10, 2024 •

edited

Loading

amoshydra Apr 10, 2024 •

edited

Loading

amoshydra commented Apr 10, 2024 •

edited

Loading

`output.preserveModules: true`