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

Do not bail on SHOW_CONFIG_FOR matches #12954

Merged
merged 6 commits into from Mar 23, 2021
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 2, 2021

Q                       A
Fixed Issues? Fixes #12425
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

I will mark this PR ready when the Windows CI is green.

The test fixtures are updated because now Babel will actually resolve presets and transform the code. It turns out the complex config in out test cases is not a valid config.

@JLHwung JLHwung added PR: Polish 💅 A type of pull request used for our changelog categories area: config labels Mar 2, 2021
@@ -125,3 +124,4 @@ programmatic options from @babel/cli
},
"filename": "./src/index.js"
}
"use strict";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should print a separator between the code and the config? Something like


================================================================================

As an alternative, maybe it would be better to print the config on stderr so that it doesn't breaks things such as

cat input.js | babel --config-file ./babel.config.json > output.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the debug output should be directed to stderr.

For example curl -v http://example.com/example.avif > example.avif will also break, unless we specify -O example.avif. Similary in this case, consider use --out-file example.js instead.

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 2, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/44385/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 2, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 46b622e:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@@ -11,7 +11,6 @@ module.exports = {
"@foo/babel-preset-1",
[
{
name: "@foo/inline-babel-preset-1",
Copy link
Member

Choose a reason for hiding this comment

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

Q: What is this change needed for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Babel will throw name is not a valid option. I didn't know name is forbidden on preset until now. 🤦‍♂️

@JLHwung JLHwung marked this pull request as ready for review March 2, 2021 22:44
Copy link
Member

@hzoo hzoo left a comment

Choose a reason for hiding this comment

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

lgtm

aside: looks like the only place we have docs on this is in https://babeljs.io/docs/en/configuration#print-effective-configs? Maybe should link to it in the options/config page as well https://babeljs.io/docs/en/config-files?
Or also suggest running when encountering certain kinds of errors

@nicolo-ribaudo
Copy link
Member

A good idea would be adding it to the issue template 👍
So that people not only can tell us what config they think they have, but also what Babel sees.

@nicolo-ribaudo nicolo-ribaudo merged commit b784c81 into babel:main Mar 23, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the fix-12425 branch March 23, 2021 19:02
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: config outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Polish 💅 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reconsider bailing out with BABEL_SHOW_CONFIG_FOR
4 participants