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 strict ESM support #40

Merged
merged 1 commit into from
Apr 9, 2024
Merged

feat!: add strict ESM support #40

merged 1 commit into from
Apr 9, 2024

Conversation

stipsan
Copy link
Member

@stipsan stipsan commented Apr 9, 2024

@snorrees working on this I realise it's time for @sanity/plugin-kit to get some love. I'll make a PR on that repo with the new best practices we've developed for semantic-release, @sanity/pkg-utils, shipping ESM, using tsconfig's new module: "Preserve" config and more 🙌

In this PR the goal is to enable strict ESM mode support. What is that? The ability to create a file like test.mjs that does this:

import { assist } from "@sanity/assist";

console.log(await import.meta.resolve("@sanity/assist"), { assist });

And have it output file:///node_modules/@sanity/assist/dist/index.mjs { assist: [Function (anonymous)] }

Today, due to our esm wrapper pattern, it outputs:

file:///node_modules/@sanity/assist/dist/index.cjs.mjs { assist: [Function (anonymous)] }

The index.cjs.mjs file is re-exporting the cjs file. In other words, it takes Node out of ESM mode and resolves @sanity/assist, and all its deps, in CJS mode.
It's not enough to simply remove the re-export pattern:

-      "node": {
-        "module": "./dist/index.esm.js",
-        "import": "./dist/index.cjs.mjs"
-      },

This will create this output

file:///test.mjs:1
import { assist } from "@sanity/assist";
         ^^^^^^
SyntaxError: Named export 'assist' not found. The requested module '@sanity/assist' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@sanity/assist';
const { assist } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:132:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:214:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async loadESM (node:internal/process/esm_loader:28:7)
    at async handleMainPromise (node:internal/modules/run_main:113:12)

Node.js v20.11.1

As you also need to use .mjs file endings:

-      "import": "./dist/index.esm.js",
+      "import": "./dist/index.mjs",

And used dependencies (like in this case date-fns) has to be updated to support native ESM, or inlined by moving them out of peerDependencies and dependencies and into devDependencies (@sanity/pkg-utils marks packages in those two lists as external).

Why do we want native ESM mode? New environments like Remix + Vite are much stricter with how it deals with ESM, and doesn't support workarounds like exports["."].node.import that re-exports CJS. To support Studios embedding in those environments we have to be strictly ESM ready.

@@ -25,6 +25,7 @@
"husky": "^8.0.3",
"lint-staged": "^13.3.0",
"prettier": "^3.2.5",
"prettier-plugin-packagejson": "^2.4.14"
"prettier-plugin-packagejson": "^2.4.14",
"rollup": "^4.14.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed for the package.config.ts typings to pass, as vitest installs a different version of rollup than the one used by @sanity/pkg-utils

@@ -6,9 +6,6 @@ export default defineConfig({
dist: 'dist',
tsconfig: 'tsconfig.dist.json',

// Avoid ttyl, os and other node specific deps
external: ['@sanity/mutator'],
Copy link
Member Author

Choose a reason for hiding this comment

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

This is no longer necessary as @sanity/mutator is specified in peerDependencies and thus is automatically marked as external.

@@ -42,7 +38,7 @@
"v2-incompatible.js"
],
"scripts": {
"build": "run-s clean && plugin-kit verify-package --silent && pkg-utils build --strict && pkg-utils --strict",
Copy link
Member Author

Choose a reason for hiding this comment

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

The plugin-kit verify-package command will be added back after @sanity/plugin-kit is updated to support v6 of @sanity/pkg-utils

"rxjs": "^7.8.0",
"@sanity/ui": "^2.1.0",
"date-fns": "^3.6.0",
"lodash": "^4.17.21",
Copy link
Member Author

Choose a reason for hiding this comment

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

Specifying the lodash dependency is needed for @sanity/pkg-utils to detect that it should optimize the imports for ESM and CJS targets respectively

"sanity": "^3.26",
"styled-components": "^5.2 || ^6.0.0"
"sanity": "^3.36.4",
"styled-components": "^6.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

Only v6 of styled-components supports named exports on both ESM and CJS:

import {styled} from 'styled-components'

useDocumentPresence,
useDocumentStore,
} from 'sanity'
import {useDocumentPane} from 'sanity/desk'
import {useDocumentPane} from 'sanity/structure'
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the minimum required sanity version is bumped in peerDependencies we can migrate from the deprecated /desk exports

@stipsan stipsan merged commit cb427da into main Apr 9, 2024
10 checks passed
@stipsan stipsan deleted the support-strict-ESM branch April 9, 2024 16:02
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.

1 participant