feat: convert package to pure ESM #61
Conversation
| @@ -1,17 +0,0 @@ | |||
| coverage | |||
There was a problem hiding this comment.
I've removed this file as the files field in the package is enough for NPM to determine which files should be included. To see this in action run npm pack --dry-run in the root directory.
|
|
||
| ### CDN | ||
| For CDN, you can use [unpkg](https://unpkg.com): | ||
| If you need a CDN, you can use [Skypack](https://www.skypack.dev/view/file-selector): |
There was a problem hiding this comment.
Skypack is an optimized CDN for serving NPM packages that use the ECMAScript module system. This allows them to be imported in scripts as outlined below, no globals needed!
| ``` | ||
| **NOTE** The above is experimental and subject to change. | ||
|
|
||
| ### CommonJS |
There was a problem hiding this comment.
Since CommonJS is no longer supported I have removed this section.
There was a problem hiding this comment.
When has this happened? And what will happen to clients that still want to use this lib but have not updated to modern JS?
| @@ -0,0 +1,21 @@ | |||
| import type { InitialOptionsTsJest } from 'ts-jest'; | |||
There was a problem hiding this comment.
The Jest configuration is now written in TypeScript as well!
| "main": "./lib/index.js", | ||
| "exports": "./lib/index.js", | ||
| "types": "./lib/index.d.ts", | ||
| "sideEffects": false, |
There was a problem hiding this comment.
This allows tools like WebPack to do more advanced tree shaking on the package.
| "test": "jest --watch" | ||
| }, | ||
| "dependencies": { | ||
| "tslib": "^2.4.0" |
There was a problem hiding this comment.
Since we're compiling to modern JavaScript we no longer need the shims from tslib.
There was a problem hiding this comment.
So how about non-modern js?
| "engines": { | ||
| "node": ">=14.16" | ||
| }, | ||
| "devDependencies": { |
There was a problem hiding this comment.
I managed to remove quite a bit of dependencies as we are now only using TypeScript to compile things.
| @@ -1,24 +1,14 @@ | |||
| { | |||
There was a problem hiding this comment.
I've simplified the TypeScript configuration a bit by putting only the essential configuration here. Configurations for specific tasks such as the build, linting and testing all live in their own configs that extend from the root config.
| ] | ||
| } | ||
| }, | ||
| "no-implicit-dependencies": false |
There was a problem hiding this comment.
Since this rule detects the dependencies in the tests as 'non-dev' dependencies this yields some false positives. Since TSLint is deprecated and needs to replaced I have opted to disable this for now.
There was a problem hiding this comment.
Add comment somewhere to do that then.
|
@rolandjitsu any particular reason the PR was closed? |
|
Sorry, that was unintentional. The intent was to close the dependapot ones. Re-opening. |
rolandjitsu
left a comment
rolandjitsu
left a comment
There was a problem hiding this comment.
@jonkoops I've left a few comments. Mainly around backwards compat or support for older clients and build systems.
| ] | ||
| } | ||
| }, | ||
| "no-implicit-dependencies": false |
There was a problem hiding this comment.
Add comment somewhere to do that then.
| ``` | ||
| **NOTE** The above is experimental and subject to change. | ||
|
|
||
| ### CommonJS |
There was a problem hiding this comment.
When has this happened? And what will happen to clients that still want to use this lib but have not updated to modern JS?
| For CDN, you can use [unpkg](https://unpkg.com): | ||
| If you need a CDN, you can use [Skypack](https://www.skypack.dev/view/file-selector): | ||
| ```html | ||
| <script type="module"> |
There was a problem hiding this comment.
It's probably worth mentioning https://caniuse.com/es6-module.
| "typings": "./dist/index.d.ts", | ||
| "type": "module", | ||
| "main": "./lib/index.js", | ||
| "exports": "./lib/index.js", |
There was a problem hiding this comment.
So to understand, the module, es2015 and typings have been deprecated and no longer in use by other build systems?
| "typings": "./dist/index.d.ts", | ||
| "type": "module", | ||
| "main": "./lib/index.js", | ||
| "exports": "./lib/index.js", |
There was a problem hiding this comment.
My only concern, as mentioned previously, is that clients/builders that are not modern will stop working post this change.
| "test": "jest --watch" | ||
| }, | ||
| "dependencies": { | ||
| "tslib": "^2.4.0" |
There was a problem hiding this comment.
So how about non-modern js?
|
@rolandjitsu I think I will split out this PR into a couple of smaller more reviewable ones. It's also been a while since this PR was created, so I'd like to re-evaluate the choices made here. |
|
Marking this as a draft to get back to it later when the other work has landed. |
|
@jonkoops thanks for doing that. I appreciate your effort and sorry for the delayed reply. I'm reviewing the other PRs now. |
|
I am going to split this PR into some separate ones to make things easier to review. It's also been a while since I touched this PR, so I need to get back in the changes and perhaps do some things differently. |
|
Closing this in favor of #96. |
2 participants
What kind of change does this PR introduce?
Did you add tests for your changes?
If relevant, did you update the documentation?
Summary
This converts the package into pure ESM. This means that the code is no longer distributed as anything than standard JavaScript, avoiding dual packaging hazard in the process.
This also benefits modern bundlers such as Vite, which can do more extensive optimizations on pure packages.
Does this PR introduce a breaking change?
Yes, this raises the minimum required version of Node.js to version 14. Since Node.js 12 is EOL this should not be an issue. This also drop support for all module systems beside ECMAScript.