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

Support Node ESM #3630

Merged
merged 56 commits into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
c51d4e8
Support Node ESM
snowystinger Oct 11, 2022
73b37a3
fix exports field
snowystinger Oct 12, 2022
273a6ee
Merge branch 'main' into support-esm
snowystinger Oct 12, 2022
09b34df
Change our nextjs app to esm
snowystinger Oct 12, 2022
a7a72a6
run verdaccio build on this branc
snowystinger Oct 12, 2022
fa880e1
fix typo
snowystinger Oct 12, 2022
b171e38
restore verdaccio build to main
snowystinger Oct 13, 2022
024022a
Merge branch 'main' into support-esm
snowystinger Oct 13, 2022
0199e65
Merge branch 'main' into support-esm
snowystinger Oct 13, 2022
6ef6151
add subfields
snowystinger Oct 13, 2022
5862a6d
Merge branch 'main' into support-esm
snowystinger Oct 14, 2022
1cde05e
remove module from node only pkg
snowystinger Oct 14, 2022
b97a830
restore verdaccio to main
snowystinger Oct 14, 2022
9404798
change example back to default
snowystinger Oct 14, 2022
584c5dd
Merge branch 'main' into support-esm
snowystinger Oct 17, 2022
6a3deee
Merge branch 'main' into support-esm
snowystinger Oct 18, 2022
ebc898a
fix icons and file extensions
snowystinger Oct 24, 2022
92624d2
fix storybook, enable verdaccio
snowystinger Oct 24, 2022
2c509ae
fix tests
snowystinger Oct 25, 2022
4d1018a
fix esm test
snowystinger Oct 25, 2022
90edded
fix docs build
snowystinger Oct 25, 2022
d2db962
Make diff easier to read
snowystinger Oct 25, 2022
ff7945b
remove pkg.json circular dependency
snowystinger Oct 25, 2022
89eb385
fix old lint rule
snowystinger Oct 25, 2022
26d8b5d
fix immediate circular dependencies
snowystinger Oct 25, 2022
50e162a
remove check for easier verdaccio M1
snowystinger Oct 25, 2022
d80e620
example to build against latest
snowystinger Oct 25, 2022
0506e6e
mark icon packages as cjs
snowystinger Oct 26, 2022
d5628e3
try using a babel plugin to work around default exports
snowystinger Oct 26, 2022
940d3fd
revert node update
snowystinger Oct 26, 2022
d65b6ff
make all icons support esm and cjs
snowystinger Oct 31, 2022
328c98b
Merge branch 'main' into support-esm
snowystinger Oct 31, 2022
27bf987
fix remaining icon imports with .js
snowystinger Oct 31, 2022
715c9a6
fix dependencies
snowystinger Oct 31, 2022
402cf06
fix gitignore
snowystinger Oct 31, 2022
93a9be3
fix exports identifiers
snowystinger Oct 31, 2022
9fa10b0
try expansion nested exports
snowystinger Oct 31, 2022
903523e
expansion didn't work
snowystinger Oct 31, 2022
a78345c
fix arg order
snowystinger Oct 31, 2022
a09df36
correct export again
snowystinger Oct 31, 2022
410e09c
and commit
snowystinger Nov 1, 2022
3e93ff4
fix tests
snowystinger Nov 2, 2022
0857a88
Merge branch 'main' into support-esm
snowystinger Nov 2, 2022
40f2ae4
Merge branch 'main' into support-esm
snowystinger Nov 9, 2022
ae02c75
Add types to exports
snowystinger Nov 9, 2022
e1a10f1
update plop templates
snowystinger Nov 9, 2022
372cc2b
fix express icon resolution
snowystinger Nov 9, 2022
4c8fac9
Merge branch 'main' into support-esm
snowystinger Nov 18, 2022
5dd5f72
Don't require tsdiff to publish verdaccio
snowystinger Nov 21, 2022
e0e9673
Merge branch 'main' into support-esm
snowystinger Nov 21, 2022
1815929
Revert circle-ci changes to main
snowystinger Nov 21, 2022
4ddba9c
Merge branch 'support-esm' of github.com:adobe/react-spectrum into su…
snowystinger Nov 21, 2022
6b7437b
undo whitespace change to NumberField test
snowystinger Nov 21, 2022
1f0423a
undo verdaccio cleanup change for non-m1 macs
snowystinger Nov 21, 2022
2bd119a
fix plop templates
snowystinger Nov 22, 2022
bb0b68c
Merge branch 'main' into support-esm
LFDanLu Nov 22, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .circleci/api-comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,13 @@ async function run() {

if (pr != null) {
let commentId = await findDifferComment(pr);
let diffs = fs.readFileSync('/tmp/dist/ts-diff.txt');
let diffs;
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 just to make it easier when testing verdaccio on a branch, you don't need to worry about there being a ts-diff file

try {
diffs = fs.readFileSync('/tmp/dist/ts-diff.txt');
} catch (e) {
console.log('No TS Diff output to run on.')
return;
}
if (diffs.length > 0) {
if (commentId != null) {
// delete existing comment
Expand Down
26 changes: 26 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ executors:
CACHE_VERSION: v1
working_directory: ~/react-spectrum

rsp-xlarge-nodeupdate:
docker:
- image: cimg/node:16.18.0
Copy link
Member Author

Choose a reason for hiding this comment

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

16.18 needed to test modules imports with a custom CSS loader
but I didn't want to introduce that much of a change across the board

resource_class: xlarge
environment:
CACHE_VERSION: v1
working_directory: ~/react-spectrum

jobs:
install:
executor: rsp-large
Expand Down Expand Up @@ -206,6 +214,20 @@ jobs:
- store_artifacts:
path: ~/junit


test-esm:
executor: rsp-xlarge-nodeupdate
steps:
- restore_cache:
key: react-spectrum-{{ .Environment.CACHE_VERSION }}-{{ .Environment.CIRCLE_SHA1 }}

- run:
name: test
command: |
make build
yarn lerna run prepublishOnly
node --loader ./scripts/esm-support/loader.mjs ./scripts/esm-support/testESM.mjs

lint:
executor: rsp
steps:
Expand Down Expand Up @@ -450,6 +472,9 @@ workflows:
- test-17:
requires:
- install-17
- test-esm:
requires:
- install
- lint:
requires:
- install
Expand Down Expand Up @@ -505,6 +530,7 @@ workflows:
- test-16
- test-ssr-17
- test-17
- test-esm
- storybook
- storybook-16
- storybook-17
Expand Down
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ node_modules
packages/*/*/dist
packages/react-aria/dist
packages/react-stately/dist
packages/dev/storybook-builder-preview/preview.js
packages/dev/storybook-builder-parcel/preview.js
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 the actual path and it became apparent while testing this branch locally

61 changes: 61 additions & 0 deletions babel-esm.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
{
"presets": [
"@babel/preset-typescript",
"@babel/preset-react",
["@babel/preset-env",
{
"loose": true,
"modules": false
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 the key difference between the original babel config and the esm one
the other parts might be unnecessary, but it was easier to stay as close as possible to the original

}
]
],
"env": {
"storybook": {
"presets": [
[
"@babel/preset-env",
{
"loose": true,
"targets": {
"esmodules": true
}
}
]
]
},
"cover": {
"plugins": [
"istanbul"
]
},
"production": {
"plugins": [
[
"react-remove-properties",
{
"properties": [
"data-testid"
]
}
]
]
}
},
"plugins": [
[
"@babel/plugin-transform-runtime",
{
"version": "^7.6.2"
}
],
[
"@babel/plugin-proposal-decorators",
{
"legacy": true
}
],
"transform-glob-import",
"babel-plugin-macros"
],
"sourceType": "unambiguous"
}
137 changes: 71 additions & 66 deletions bin/imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,73 +16,78 @@ const fs = require('fs');
const Module = require('module');
const substrings = ['-', '+'];

module.exports = function (context) {
let processNode = (node) => {
if (!node.source || node.importKind === 'type') {
return;
}

let source = node.source.value.replace(/^[a-z]+:/, '');
if (source.startsWith('.') || Module.builtinModules.includes(source)) {
return;
}

// Split the import specifier on slashes. If it starts with an @ then it's
// a scoped package, otherwise just take the first part.
let parts = source.split('/');
let pkgName = source.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0];

// Search for a package.json starting from the current filename
let pkgPath = findUp.sync('package.json', {cwd: path.dirname(context.getFilename())});
if (!pkgPath) {
return;
}

let pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8'));

// The only dev dependency should be spectrum-css.
if (exists(pkg.devDependencies, pkgName) && pkgName === '@adobe/spectrum-css-temp') {
return;
}

if (!exists(pkg.dependencies, pkgName) && !exists(pkg.peerDependencies, pkgName)) {
context.report({
node,
message: `Missing dependency on ${pkgName}.`,
fix(fixer) {
// Attempt to find a package in the monorepo. If the dep is for an external library,
// then we cannot auto fix it because we don't know the version to add.
let depPath = __dirname + '/../packages/' + pkgName + '/package.json';
if (!fs.existsSync(depPath)) {
return;
module.exports = {
meta: {
fixable: 'code'
},
create: function (context) {
Copy link
Member Author

Choose a reason for hiding this comment

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

no actual change to this logic, it's just indented one level and wrapped differently to match new eslint requirements

let processNode = (node) => {
if (!node.source || node.importKind === 'type') {
return;
}

let source = node.source.value.replace(/^[a-z]+:/, '');
if (source.startsWith('.') || Module.builtinModules.includes(source)) {
return;
}

// Split the import specifier on slashes. If it starts with an @ then it's
// a scoped package, otherwise just take the first part.
let parts = source.split('/');
let pkgName = source.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0];

// Search for a package.json starting from the current filename
let pkgPath = findUp.sync('package.json', {cwd: path.dirname(context.getFilename())});
if (!pkgPath) {
return;
}

let pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8'));

// The only dev dependency should be spectrum-css.
if (exists(pkg.devDependencies, pkgName) && pkgName === '@adobe/spectrum-css-temp') {
return;
}

if (!exists(pkg.dependencies, pkgName) && !exists(pkg.peerDependencies, pkgName) && pkgName !== pkg.name) {
context.report({
node,
message: `Missing dependency on ${pkgName}.`,
fix(fixer) {
// Attempt to find a package in the monorepo. If the dep is for an external library,
// then we cannot auto fix it because we don't know the version to add.
let depPath = __dirname + '/../packages/' + pkgName + '/package.json';
if (!fs.existsSync(depPath)) {
return;
}

let depPkg = JSON.parse(fs.readFileSync(depPath, 'utf8'));
let pkgVersion = substrings.some(v => depPkg.version.includes(v)) ? depPkg.version : `^${depPkg.version}`;

if (pkgName === '@react-spectrum/provider') {
pkg.peerDependencies = insertObject(pkg.peerDependencies, pkgName, pkgVersion);
} else {
pkg.dependencies = insertObject(pkg.dependencies, pkgName, pkgVersion);
}

fs.writeFileSync(pkgPath, JSON.stringify(pkg, false, 2) + '\n');

// Fake fix so eslint doesn't show the error.
return {
range: [0, 0],
text: ''
};
}

let depPkg = JSON.parse(fs.readFileSync(depPath, 'utf8'));
let pkgVersion = substrings.some(v => depPkg.version.includes(v)) ? depPkg.version : `^${depPkg.version}`;

if (pkgName === '@react-spectrum/provider') {
pkg.peerDependencies = insertObject(pkg.peerDependencies, pkgName, pkgVersion);
} else {
pkg.dependencies = insertObject(pkg.dependencies, pkgName, pkgVersion);
}

fs.writeFileSync(pkgPath, JSON.stringify(pkg, false, 2) + '\n');

// Fake fix so eslint doesn't show the error.
return {
range: [0, 0],
text: ''
};
}
});
}
};

return {
ImportDeclaration: processNode,
ExportNamedDeclaration: processNode,
ExportAllDeclaration: processNode
};
});
}
};

return {
ImportDeclaration: processNode,
ExportNamedDeclaration: processNode,
ExportAllDeclaration: processNode
};
}
};

function exists(deps, name) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const withTM = require("next-transpile-modules")([
import ntm from 'next-transpile-modules';
const withTM = ntm([
"@adobe/react-spectrum",
"@react-spectrum/actiongroup",
"@react-spectrum/badge",
Expand Down Expand Up @@ -51,9 +52,10 @@ const withTM = require("next-transpile-modules")([
"@spectrum-icons/workflow",
]);

module.exports = withTM({
let result = withTM({
basePath:
process.env.VERDACCIO && process.env.CIRCLE_SHA1
? `/reactspectrum/${process.env.CIRCLE_SHA1}/verdaccio/next`
: "",
});
export default result;
snowystinger marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions examples/rsp-next-ts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"start": "next start",
"lint": "next lint"
},
"type": "module",
"dependencies": {
"@adobe/react-spectrum": "^3.22.0",
"@react-spectrum/color": "^3.0.0-beta.16",
Expand Down
Loading