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

feat: add ESM export #13

Closed
wants to merge 1 commit into from
Closed

feat: add ESM export #13

wants to merge 1 commit into from

Conversation

stipsan
Copy link
Member

@stipsan stipsan commented Jun 8, 2022

Adds module and exports to package.json.
The files referenced are generated using esbuild.

What this solves

A lot of new, modern, environments require packages to start shipping ESM.
For example when attempting to use @sanity/client in a Remix site will yield an error like the one below:
image
This is from same-origin, which is included by get-it, which is used by @sanity/client.
To solve this there's a new build command npm run esbuild:browser that transforms CJS to ESM and includes them in the bundle. This solves the problem, for now, buying us time to migrate these packages to ESM:

Once they are shipping ESM we can stop prebundling them:

    "build": "npm run compile && npm run browserify && npm run minify && npm run esbuild:browser && npm run esbuild:node",
-   "esbuild": "esbuild src/sanityClient.js --bundle --sourcemap",
+   "esbuild": "esbuild src/sanityClient.js --bundle --sourcemap --external:rxjs --external:@sanity/eventsource --external:get-it --external:make-error --external:@sanity/generate-help-url",
    "esbuild:browser": "npm run esbuild -- --format=esm --outfile=dist/sanityClient.browser.mjs --platform=browser",
-   "esbuild:node": "npm run esbuild -- --format=cjs --outfile=dist/sanityClient.node.cjs --platform=node --external:object-assign --external:rxjs --external:@sanity/eventsource --external:get-it --external:make-error --external:@sanity/generate-help-url",
+   "esbuild:node": "npm run esbuild -- --format=cjs --outfile=dist/sanityClient.node.cjs --platform=node",
    "lint": "eslint .",

How to test

A test version is published under npm i @sanity/client@esm.

Using unpkg.com

Run this in your console (not on github as it blocks external scripts, but for example on https://sanity.io or a localhost):

const {default: createClient} = await import('https://unpkg.com/@sanity/client@esm?module=true')
createClient() // Should throw as no projectId is given

Using skypack.dev

const {default: createClient} = await import('https://cdn.skypack.dev/@sanity/client@esm')
createClient() // Should throw as no projectId is given

./dist/sanityClient.browser.mjs

This file is generated to run in ESM strict envs, like deno, native browser ESM using CDNs like unpkg or skypack. To do this it bundles everything (no import or require calls left) and it applies our pkg.browser field making sure to load browserMiddleware.js instead of nodeMiddleware.js.

./dist/sanityClient.node.cjs

Generated to run in allows calling require, using .cjs to make it clear to modern 'node' that it's a CommonJS file.

Once we have a bundler that implements module.createRequire we could consider adding a ./dist/sanityClient.node.mjs version.

pkg.module

This only affects bundlers that are modern enough to understand what it does, and does not trigger modern node to run in ESM module (unlike pkg.type: "module"). It points to dist/sanityClient.browser.mjs instead of dist/sanityClient.node.js as it has the highest chance of bundling success since pkg.browser is already applied to it.

adding package-check to posttest

Runs @skypack/package-check to validate we're following ESM best practices.

@stipsan stipsan requested a review from rexxars June 8, 2022 11:37
@@ -8,6 +8,6 @@
"browser": true
},
"parserOptions": {
"ecmaVersion": 2018
"ecmaVersion": 2020
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes a parse error due to the dynamic import() in client.test.js

@@ -8,7 +8,7 @@ jobs:

strategy:
matrix:
node-version: [12.x, 14.x, 16.x, 18.x]
Copy link
Member Author

Choose a reason for hiding this comment

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

node v12 is dead, longe live node v12 👑

@stipsan stipsan mentioned this pull request Jun 9, 2022
@rexxars rexxars removed their request for review August 24, 2022 22:19
@stipsan stipsan closed this Aug 26, 2022
@stipsan stipsan deleted the fix-esm branch August 26, 2022 19:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant