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

Pass version to presetEnv #5946

Closed

Conversation

bemnlam
Copy link

@bemnlam bemnlam commented Mar 2, 2021

↪️ Pull Request

Fixing #5943

This change in babel breaks Parcel if version doesn't pass into presetEnv(). By setting version as 7.13.0 presetEnv() will continue other checking, just like the logic before.

This change in babel babel/babel#12934 breaks Parcel if version doesn't pass into presetEnv(). By setting version as 7.13.0 presetEnv() will continue other checking, just like the logic before.
@height
Copy link

height bot commented Mar 2, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@devongovett
Copy link
Member

I think this is a v1 only bug? This code path won't actually be hit in v2 because config.isSource is always true.

@nicolo-ribaudo
Copy link

Q: If that code path won't be hit, the code can just be deleted?

@@ -48,7 +48,7 @@ export default async function getEnvOptions(

function getNeededPlugins(targets: BabelTargets): Array<PresetEnvPlugin> {
return presetEnv(
{assertVersion: () => true},
{version: '7.13.0', assertVersion: () => true}, // version is required since https://github.com/babel/babel/pull/12934/

Choose a reason for hiding this comment

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

This should be 7.12.0 (

"@babel/core": "^7.12.0"
), or require("@babel/core").version.

Ideally, the safest approach would be { ...require("@babel/core"), assertVersion: () => true }.

@mischnic
Copy link
Member

mischnic commented Mar 2, 2021

Yes,

// Don't transpile inside node_modules
if (!config.isSource) {
return;
}

means that this whole if statement could be removed because it's never reached if !config.isSource:

// If this is the app module, the source and target will be the same, so just compile everything.
// Otherwise, load the source engines and generate a babel-present-env config.
if (!config.isSource) {
let sourceBabelTargets = await getBabelTargets(config);
if (
!sourceBabelTargets ||
!shouldCompileFurther(sourceBabelTargets, appBabelTargets)
) {
return null;
}
}

@bemnlam
Copy link
Author

bemnlam commented Mar 3, 2021

@nicolo-ribaudo @mischnic so looks like this is a v1-only issue and what can I do if I also want to apply this change to v1? (sorry I am a newbie and I want to help 🙏)

@nicolo-ribaudo
Copy link

@bemnlam Parcel 1 is on the master branch, but I'd wait for the ok from a maintainer before opening a PR for Parcel 1.

@agilgur5
Copy link

@devongovett @mischnic could y'all review this change, particularly for v1? #5943 is being felt quite massively across the community as I'm sure you're seeing. I was going to write this PR myself (even though I'm only indirectly impacted) before finding it was already done here.

Currently, the only solution right now is either a resolution or a not-so-easy migration to the v2 beta -- this fix is considerably simpler than either.

@devongovett
Copy link
Member

v1.12.5 has a fix. Please see #5943 (comment)

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.

5 participants