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.

Sign up for GitHub

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

Jump to bottom

Resolve babel.config.js 'babelrcRoots' values relative to the config file. #8910

Merged
merged 1 commit into from Nov 5, 2018
Merged

Resolve babel.config.js 'babelrcRoots' values relative to the config file. #8910

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 11 additions & 4 deletions packages/babel-core/src/config/config-chain.js
View file
Open in desktop
Expand Up @@ -155,6 +155,7 @@ export function buildRootChain(
}

let { babelrc, babelrcRoots } = opts;
let babelrcRootsDirectory = context.cwd;

const configFileChain = emptyChain();
if (configFile) {
Expand All @@ -168,6 +169,7 @@ export function buildRootChain(
babelrc = validatedFile.options.babelrc;
}
if (babelrcRoots === undefined) {
babelrcRootsDirectory = validatedFile.dirname;
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain me how this PR fixes the issue?
If I understand it correctly, this line is the only change: if babelrcRoots undefined, we use validatedFile.dirname instead of context.cwd, which is the default value of babelrcRootsDirectory (which is the same as the old behavior).
In the issue you said that it occurs when babelrcRoots is set to something, so I don't understand how this PR fixes it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue is that users may be doing configFile: "some/nested/path/config.js" for instance, or in cases like

project/
  babel.config.js
  packages/
    module/
      // run Babel with cwd here with `rootMode: "upward"`

and in both cases, previously we'd use cwd for the paths, rather than the location of the config file, so if your babelrcRoots value was

babelrcRoots: [
  ".",
]

the expectation is that the . is relative to the config file, but currently we are calculating it relative to the working directory.

Copy link
Member

Choose a reason for hiding this comment

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

babelrcRoots: [
   ".",
]

the expectation is that the . is relative to the config file, but currently we are calculating it relative to the working directory.

But doesn't this PR only changes the resolution behavior if babelrcRoots === undefined? If it is an array we still resolve relative to context.cwd, because we never go inside that if.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, the if (babelrcRoots === undefined) { check here is if it was undefined in the babel.transform arguments, not if it was undefined in the config file.

With the way things are set up now, we entirely ignore babelrcRoots in the config file if it is passed in the programmatic arguments.

Copy link
Member

Choose a reason for hiding this comment

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

Oh ok 👍

babelrcRoots = validatedFile.options.babelrcRoots;
}

Expand All @@ -185,7 +187,7 @@ export function buildRootChain(
if (
(babelrc === true || babelrc === undefined) &&
pkgData &&
babelrcLoadEnabled(context, pkgData, babelrcRoots)
babelrcLoadEnabled(context, pkgData, babelrcRoots, babelrcRootsDirectory)
) {
({ ignore: ignoreFile, config: babelrcFile } = findRelativeConfig(
pkgData,
Expand Down Expand Up @@ -229,6 +231,7 @@ function babelrcLoadEnabled(
context: ConfigContext,
pkgData: FilePackageData,
babelrcRoots: BabelrcSearch | void,
babelrcRootsDirectory: string,
): boolean {
if (typeof babelrcRoots === "boolean") return babelrcRoots;

Expand All @@ -243,7 +246,9 @@ function babelrcLoadEnabled(
let babelrcPatterns = babelrcRoots;
if (!Array.isArray(babelrcPatterns)) babelrcPatterns = [babelrcPatterns];
babelrcPatterns = babelrcPatterns.map(pat => {
return typeof pat === "string" ? path.resolve(context.cwd, pat) : pat;
return typeof pat === "string"
? path.resolve(babelrcRootsDirectory, pat)
: pat;
});

// Fast path to avoid having to match patterns if the babelrc is just
Expand All @@ -253,10 +258,12 @@ function babelrcLoadEnabled(
}

return babelrcPatterns.some(pat => {
if (typeof pat === "string") pat = pathPatternToRegex(pat, context.cwd);
if (typeof pat === "string") {
pat = pathPatternToRegex(pat, babelrcRootsDirectory);
}

return pkgData.directories.some(directory => {
return matchPattern(pat, context.cwd, directory, context);
return matchPattern(pat, babelrcRootsDirectory, directory, context);
});
});
}
Expand Down