-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Support Node ESM #3630
Changes from 54 commits
c51d4e8
73b37a3
273a6ee
09b34df
a7a72a6
fa880e1
b171e38
024022a
0199e65
6ef6151
5862a6d
1cde05e
b97a830
9404798
584c5dd
6a3deee
ebc898a
92624d2
2c509ae
4d1018a
90edded
d2db962
ff7945b
89eb385
26d8b5d
50e162a
d80e620
0506e6e
d5628e3
940d3fd
d65b6ff
328c98b
27bf987
715c9a6
402cf06
93a9be3
9fa10b0
903523e
a78345c
a09df36
410e09c
3e93ff4
0857a88
40f2ae4
ae02c75
e1a10f1
372cc2b
4c8fac9
5dd5f72
e0e9673
1815929
4ddba9c
6b7437b
1f0423a
2bd119a
bb0b68c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,14 @@ executors: | |
CACHE_VERSION: v1 | ||
working_directory: ~/react-spectrum | ||
|
||
rsp-xlarge-nodeupdate: | ||
docker: | ||
- image: cimg/node:16.18.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 16.18 needed to test modules imports with a custom CSS loader |
||
resource_class: xlarge | ||
environment: | ||
CACHE_VERSION: v1 | ||
working_directory: ~/react-spectrum | ||
|
||
jobs: | ||
install: | ||
executor: rsp-large | ||
|
@@ -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: | ||
|
@@ -450,6 +472,9 @@ workflows: | |
- test-17: | ||
requires: | ||
- install-17 | ||
- test-esm: | ||
requires: | ||
- install | ||
- lint: | ||
requires: | ||
- install | ||
|
@@ -505,6 +530,7 @@ workflows: | |
- test-16 | ||
- test-ssr-17 | ||
- test-17 | ||
- test-esm | ||
- storybook | ||
- storybook-16 | ||
- storybook-17 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} | ||
] | ||
], | ||
"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" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
There was a problem hiding this comment.
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