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

fix: Don't load browserslist in block-hoist-plugin #13182

Merged
merged 3 commits into from Apr 21, 2021
Merged

fix: Don't load browserslist in block-hoist-plugin #13182

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
510cb1e
fix: Don't load browserslist in block-hoist-plugin
Apr 20, 2021
33d55f7
Remove test case
Apr 21, 2021
0e11cca
Remove added new line
Apr 21, 2021
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
6 changes: 6 additions & 0 deletions packages/babel-core/src/transformation/block-hoist-plugin.ts
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ export default function loadBlockHoistPlugin(): Plugin {
const config = loadConfig.sync({
babelrc: false,
configFile: false,
browserslistConfigFile: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we can completely avoid loadConfig.sync. I drafted some perf changes before but never come up with a PR: JLHwung@15da88e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would even be better! It wasn't clear to me why it was needed so I tried to work around it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JLHwung are you planning to submit your changes as a PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, feel free to tackle it.

Copy link
Member

Choose a reason for hiding this comment

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

seems good as temp fix then? didn't even know we had this..

plugins: [blockHoistPlugin],
});
LOADED_PLUGIN = config ? config.passes[0][0] : undefined;
Expand All @@ -20,6 +21,11 @@ export default function loadBlockHoistPlugin(): Plugin {

return LOADED_PLUGIN;
}

export function resetBlockHoistPlugin(): void {
LOADED_PLUGIN = undefined;
}

function priority(bodyNode) {
const priority = bodyNode?._blockHoist;
if (priority == null) return 1;
Expand Down
34 changes: 34 additions & 0 deletions packages/babel-core/test/api.js
View file
Open in desktop
Expand Up @@ -8,6 +8,8 @@ import { fileURLToPath } from "url";
import presetEnv from "../../babel-preset-env";
import pluginSyntaxFlow from "../../babel-plugin-syntax-flow";
import pluginFlowStripTypes from "../../babel-plugin-transform-flow-strip-types";
import * as helperCompilationTargets from "../../babel-helper-compilation-targets";
import { resetBlockHoistPlugin } from "../lib/transformation/block-hoist-plugin";

const cwd = path.dirname(fileURLToPath(import.meta.url));

Expand Down Expand Up @@ -172,6 +174,38 @@ describe("api", function () {
expect(options).toEqual({ babelrc: false });
});

describe("browserslistConfigFile", function () {
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
afterEach(function () {
jest.restoreAllMocks();
});

it("passes `ignoreBrowserslistConfig` to getTargets when browserslistConfigFile is false", function () {
const realGetTargets = helperCompilationTargets.default;
const getTargets = jest
.spyOn(helperCompilationTargets, "default")
.mockImplementation((inputTargets, options) => {
if (options.ignoreBrowserslistConfig !== true) {
throw new Error(
`getTargets should not be called with ignoreBrowserslistConfig set to a value other than true but called with '${options.ignoreBrowserslistsConfig}'`,
);
}

return realGetTargets(inputTargets, options);
});

const options = {
babelrc: false,
browserslistConfigFile: false,
};
resetBlockHoistPlugin(); // Force reload the plugin

Object.freeze(options);
transformFileSync(cwd + "/fixtures/api/file.js", options);

expect(getTargets).toHaveBeenCalled();
});
});

it("transformFromAst should not mutate the AST", function () {
const program = "const identifier = 1";
const node = parse(program);
Expand Down