-
Notifications
You must be signed in to change notification settings - Fork 277
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
fix(father-build): babel runtime resolve logic #484
Conversation
Codecov Report
@@ Coverage Diff @@
## master #484 +/- ##
=======================================
Coverage 75.31% 75.31%
=======================================
Files 18 18
Lines 559 559
Branches 199 199
=======================================
Hits 421 421
Misses 137 137
Partials 1 1
Continue to review full report at Codecov.
|
@@ -35,6 +36,14 @@ function transformImportLess2Css() { | |||
} | |||
} | |||
|
|||
function getBabelRuntimeVersion(cwd: string) { | |||
try { | |||
return require(require.resolve('@babel/runtime/package.json', { paths: [cwd] })).version; |
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.
It would be better to check what depencensies["@babel/runtime"]
says. If someone, for example, specifies ^7.0.0
they could have 7.18.0
in their node_modules, however the generated code should be compatible with 7.0.0 because their users might have a version older than 7.18.0.
This is what's causing the current problems, and I don't think that this PR would fix them.
@@ -81,7 +82,8 @@ export default function(opts: IGetBabelConfigOpts) { | |||
...(runtimeHelpers | |||
? [[require.resolve('@babel/plugin-transform-runtime'), { | |||
useESModules: isBrowser && (type === 'esm'), | |||
version: require('@babel/runtime/package.json').version, | |||
// use @babel/runtime version from project dependencies | |||
version: require(`${opts.cwd}/package.json`).dependencies['@babel/runtime'], |
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.
I suggest using try-catch with a fallback to undefined
:
function getBabelRuntimeVersion(cwd) {
try {
return require(`${cwd}/package.json`).dependencies['@babel/runtime']
} catch {}
}
This is because that require call might fail, for example if the user as an uncommon setup or if for some reason they put @babel/runtime
in a different package.json
.
By returning undefined
in the error case, Babel will automatically output code that is compatible with every version starting from 7.0.0.
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.
It was removed on purpose, because there has a validation logic before this and I forgot just now, at:
father/packages/father-build/src/build.ts
Lines 50 to 55 in e38134e
assert.ok(existsSync(pkgPath), `@babel/runtime dependency is required to use runtimeHelpers`); | |
const pkg = JSON.parse(readFileSync(pkgPath, 'utf-8')); | |
assert.ok( | |
(pkg.dependencies || {})['@babel/runtime'], | |
`@babel/runtime dependency is required to use runtimeHelpers` | |
); |
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.
Thank you very much for your detailed comment ❤️
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.
Oh ok, that's good!
Thank you very much for your detailed comment ❤️
I'm happy to help 😄
Use project
@babel/runtime
firstref: ant-design/pro-components#5273