-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
add showConfig support #11588
add showConfig support #11588
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/26507/ |
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 a577902:
|
6089574
to
56516c5
Compare
6cacd17
to
46978e8
Compare
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 awesome
|
||
mergeChain(fileChain, result); | ||
} | ||
} | ||
|
||
if (context.showConfig) { | ||
console.log( | ||
// print config by the order of descending priority |
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.
Maybe we should print in the output that in case of conflicts the first config overrides the second?
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 tend to keep the output succinct. The implementation detail of config merging should be referenced from babel docs but not a debug output.
To reason how these config merges, we can print the resolved config in later PRs. Think of the Styles
tab v.s. the Computed
tab in the Chrome DevTools / Elements.
} else if (typeof d.value === "function") { | ||
// If the unloaded descriptor is a function, i.e. `plugins: [ require("my-plugin") ]`, | ||
// we print the first 50 characters of the function source code and hopefully we can see | ||
// `name: 'my-plugin'` in the source |
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.
Good hack, although I don't believe this will be the case for most plugins?
Even testing on https://codesandbox.io/s/lively-star-xv8bt, it will just print the code instead, the default plugin on ASTExplorer wouldn't even show the name until char 85.
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.
Good catch. Well since an object has no key orders, I don't think there exists an integer N such that the plugin name is included in the (0, N)
substring.
My initial thought here is to provide user at least something more descriptive/recognizable than a generic [Function]
string. I have come up with several approach:
- Print Function name. It could not help much because we don't have such convention that the default export should be a named function
- Use regex to extract
name: "blablabla"
from function source. It doesn't work with dynamic name and has false positive. - Actually load the descriptor. We have to initialize the API object earlier, and it may introduce breaking changes as we can end up loading a plugin twice.
So I end up with this approach and hope it could serve as an indicator so that users could know what this plugin is -- most of the time these are ad-hoc plugins not in node_modules
.
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.
Use regex to extract name: "blablabla" from function source. It doesn't work with dynamic name
I think it would be really rare/weird to have a dynamic name, it's just a string? Yeah I don't think it'll be an issue in general but for standalone/repl which is a diff usecase altogether.
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 think that this is something that we can revisit with experience: I believe that the hard-to-understand function name might not be a problem for a few reasons:
- Most people use
"babel-plugin-foo"
orrequire.resolve("babel-plugin-foo")
.slice(0, 50)
always includes the plugin function name (or at least the first 41 characters if it's longer). This can be useful for plugins directly defined in the config file, as we do.- If the plugin is an actual anonymous function and the
.slice(0, 50)
substring doesn't contain any useful information, the person can easily see in which config file it is defined and check what's used at that index in theplugins
array directly in the config file.
c82e049
to
bf61000
Compare
c787103
to
c6b74c8
Compare
export function* resolveShowConfigPath( | ||
dirname: string, | ||
): Handler<string | null> { | ||
const targetPath = process.env.BABEL_SHOW_CONFIG_FOR_PATH; |
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.
Do you think the following candidates are easier for typing?
BABEL_SHOW_CONFIG_FOR
BABEL_CONFIG_FOR
I don't think we can simply drop FOR
because it looks like a boolean toggle
BABEL_SHOW_CONFIG=true
will never work as intended.
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 slightly prefer BABEL_SHOW_CONFIG_FOR
edbe64d
to
3886071
Compare
Should we add a test for .only? |
Co-authored-by: Brian Ng <bng412@gmail.com>
7c1c2ac
to
0d5fe2c
Compare
Just a follow up: is there a reason you removed (bfdf5cf) the showConfig boolean option? That seemed like the easiest way to use this feature. |
The reason is that it would log the config for every file processed by Babel, generating an output so big that it would hardly be useful. |
showConfig
will print the config chain applied on a specific file path by order of descending priority. It will also print the effectiveoverrides
andenv
config applied to a single file.Example output (input, Full output)
(The
<CWD>
and<ROOTDIR>
is not presented in the raw output, it is replaced by Babel's fixture runner.)Known limits
presets
These limits can be addressed in other PRs.
Additional
This PR includes commits in #11544.