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

Setup JSX runtime even if the file doesn't contain JSX #12479

Merged
merged 2 commits into from Dec 10, 2020
Merged

Setup JSX runtime even if the file doesn't contain JSX #12479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67c163d
Setup JSX runtime even if the file doesn't contain JSX
nicolo-ribaudo Dec 10, 2020
2cbe36d
Node 6
nicolo-ribaudo Dec 10, 2020
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
244 changes: 115 additions & 129 deletions packages/babel-helper-builder-react-jsx-experimental/src/index.js
View file
Open in desktop
Expand Up @@ -138,136 +138,134 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,

Program: {
enter(path, state) {
if (hasJSX(path)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming the reason why this was done originally was for performance? And that there aren't plugins that convert non-jsx stuff but that I guess isn't the case then

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah probably for performance, but doing a deep traversal to check if there is JSX isn't going to be fast anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Makes me think this sort of thing shouldn't need to be set every file if it's the same for all of them (for the most part unless you use comments or overrides)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, usually you just set it once in the config file.

const { file } = state;
let runtime = RUNTIME_DEFAULT;

// For jsx mode
let source = IMPORT_SOURCE_DEFAULT;
let sourceSet = !!options.importSource;

// For createElement mode
let pragma = PRAGMA_DEFAULT;
let pragmaFrag = PRAGMA_FRAG_DEFAULT;
let pragmaSet = !!options.pragma;
let pragmaFragSet = !!options.pragmaFrag;

if (file.ast.comments) {
for (const comment of (file.ast.comments: Array<Object>)) {
const sourceMatches = JSX_SOURCE_ANNOTATION_REGEX.exec(
comment.value,
);
if (sourceMatches) {
source = sourceMatches[1];
sourceSet = true;
}

const runtimeMatches = JSX_RUNTIME_ANNOTATION_REGEX.exec(
comment.value,
);
if (runtimeMatches) {
runtime = runtimeMatches[1];
}

const jsxMatches = JSX_ANNOTATION_REGEX.exec(comment.value);
if (jsxMatches) {
pragma = jsxMatches[1];
pragmaSet = true;
}
const jsxFragMatches = JSX_FRAG_ANNOTATION_REGEX.exec(
comment.value,
);
if (jsxFragMatches) {
pragmaFrag = jsxFragMatches[1];
pragmaFragSet = true;
}
const { file } = state;
let runtime = RUNTIME_DEFAULT;

// For jsx mode
let source = IMPORT_SOURCE_DEFAULT;
let sourceSet = !!options.importSource;

// For createElement mode
let pragma = PRAGMA_DEFAULT;
let pragmaFrag = PRAGMA_FRAG_DEFAULT;
let pragmaSet = !!options.pragma;
let pragmaFragSet = !!options.pragmaFrag;

if (file.ast.comments) {
for (const comment of (file.ast.comments: Array<Object>)) {
const sourceMatches = JSX_SOURCE_ANNOTATION_REGEX.exec(
comment.value,
);
if (sourceMatches) {
source = sourceMatches[1];
sourceSet = true;
}
}

state.set("@babel/plugin-react-jsx/runtime", runtime);
if (runtime === "classic") {
if (sourceSet) {
throw path.buildCodeFrameError(
`importSource cannot be set when runtime is classic.`,
);
}
state.set(
"@babel/plugin-react-jsx/createElementIdentifier",
createIdentifierParser(pragma),
const runtimeMatches = JSX_RUNTIME_ANNOTATION_REGEX.exec(
comment.value,
);
state.set(
"@babel/plugin-react-jsx/jsxFragIdentifier",
createIdentifierParser(pragmaFrag),
);
state.set("@babel/plugin-react-jsx/usedFragment", false);
state.set(
"@babel/plugin-react-jsx/pragmaSet",
pragma !== DEFAULT.pragma,
);
state.set(
"@babel/plugin-react-jsx/pragmaFragSet",
pragmaFrag !== DEFAULT.pragmaFrag,
if (runtimeMatches) {
runtime = runtimeMatches[1];
}

const jsxMatches = JSX_ANNOTATION_REGEX.exec(comment.value);
if (jsxMatches) {
pragma = jsxMatches[1];
pragmaSet = true;
}
const jsxFragMatches = JSX_FRAG_ANNOTATION_REGEX.exec(
comment.value,
);
} else if (runtime === "automatic") {
if (pragmaSet || pragmaFragSet) {
throw path.buildCodeFrameError(
`pragma and pragmaFrag cannot be set when runtime is automatic.`,
);
if (jsxFragMatches) {
pragmaFrag = jsxFragMatches[1];
pragmaFragSet = true;
}
}
}

const importName = addAutoImports(path, {
...state.opts,
source,
});

state.set(
"@babel/plugin-react-jsx/jsxIdentifier",
createIdentifierParser(
createIdentifierName(
path,
options.development ? "jsxDEV" : "jsx",
importName,
),
),
state.set("@babel/plugin-react-jsx/runtime", runtime);
if (runtime === "classic") {
if (sourceSet) {
throw path.buildCodeFrameError(
`importSource cannot be set when runtime is classic.`,
);
state.set(
"@babel/plugin-react-jsx/jsxStaticIdentifier",
createIdentifierParser(
createIdentifierName(
path,
options.development ? "jsxDEV" : "jsxs",
importName,
),
),
}
state.set(
"@babel/plugin-react-jsx/createElementIdentifier",
createIdentifierParser(pragma),
);
state.set(
"@babel/plugin-react-jsx/jsxFragIdentifier",
createIdentifierParser(pragmaFrag),
);
state.set("@babel/plugin-react-jsx/usedFragment", false);
state.set(
"@babel/plugin-react-jsx/pragmaSet",
pragma !== DEFAULT.pragma,
);
state.set(
"@babel/plugin-react-jsx/pragmaFragSet",
pragmaFrag !== DEFAULT.pragmaFrag,
);
} else if (runtime === "automatic") {
if (pragmaSet || pragmaFragSet) {
throw path.buildCodeFrameError(
`pragma and pragmaFrag cannot be set when runtime is automatic.`,
);
}

state.set(
"@babel/plugin-react-jsx/createElementIdentifier",
createIdentifierParser(
createIdentifierName(path, "createElement", importName),
const importName = addAutoImports(path, {
...state.opts,
source,
});

state.set(
"@babel/plugin-react-jsx/jsxIdentifier",
createIdentifierParser(
createIdentifierName(
path,
options.development ? "jsxDEV" : "jsx",
importName,
),
);

state.set(
"@babel/plugin-react-jsx/jsxFragIdentifier",
createIdentifierParser(
createIdentifierName(path, "Fragment", importName),
),
);
state.set(
"@babel/plugin-react-jsx/jsxStaticIdentifier",
createIdentifierParser(
createIdentifierName(
path,
options.development ? "jsxDEV" : "jsxs",
importName,
),
);
),
);

state.set(
"@babel/plugin-react-jsx/importSourceSet",
source !== DEFAULT.importSource,
);
} else {
throw path.buildCodeFrameError(
`Runtime must be either "classic" or "automatic".`,
);
}
state.set(
"@babel/plugin-react-jsx/createElementIdentifier",
createIdentifierParser(
createIdentifierName(path, "createElement", importName),
),
);

if (options.development) {
path.traverse(injectMetaPropertiesVisitor, state);
}
state.set(
"@babel/plugin-react-jsx/jsxFragIdentifier",
createIdentifierParser(
createIdentifierName(path, "Fragment", importName),
),
);

state.set(
"@babel/plugin-react-jsx/importSourceSet",
source !== DEFAULT.importSource,
);
} else {
throw path.buildCodeFrameError(
`Runtime must be either "classic" or "automatic".`,
);
}

if (options.development) {
path.traverse(injectMetaPropertiesVisitor, state);
}
},

Expand Down Expand Up @@ -353,18 +351,6 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
return imports;
}

function hasJSX(parentPath) {
let fileHasJSX = false;
parentPath.traverse({
"JSXElement|JSXFragment"(path) {
fileHasJSX = true;
path.stop();
},
});

return fileHasJSX;
}

function getSource(source, importName) {
switch (importName) {
case "Fragment":
Expand Down
1 change: 1 addition & 0 deletions .../babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-automatic/input.js
View file
Open in desktop
@@ -0,0 +1 @@
const foo = 2;
6 changes: 6 additions & 0 deletions ...el-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-automatic/options.json
View file
Open in desktop
@@ -0,0 +1,6 @@
{
"plugins": [
["transform-react-jsx", { "runtime": "automatic" }],
"./plugin.js"
]
}
1 change: 1 addition & 0 deletions ...babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-automatic/output.js
View file
Open in desktop
@@ -0,0 +1 @@
const foo = /*#__PURE__*/undefined.jsx("p", {});
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a different unrelated bug: #12467

I'm preparing another PR for it, but it's lower priority since it's not a regression.

13 changes: 13 additions & 0 deletions ...babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-automatic/plugin.js
View file
Open in desktop
@@ -0,0 +1,13 @@
module.exports = ({ types: t }) => ({
visitor: {
NumericLiteral(path) {
path.replaceWith(
t.jsxElement(
t.jsxOpeningElement(t.jsxIdentifier("p"), []),
t.jsxClosingElement(t.jsxIdentifier("p")),
[]
)
);
}
}
});
1 change: 1 addition & 0 deletions ...es/babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-classic/input.js
View file
Open in desktop
@@ -0,0 +1 @@
const foo = 2;
6 changes: 6 additions & 0 deletions ...abel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-classic/options.json
View file
Open in desktop
@@ -0,0 +1,6 @@
{
"plugins": [
["transform-react-jsx", { "runtime": "classic" }],
"./plugin.js"
]
}
1 change: 1 addition & 0 deletions ...s/babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-classic/output.js
View file
Open in desktop
@@ -0,0 +1 @@
const foo = /*#__PURE__*/React.createElement("p", null);
13 changes: 13 additions & 0 deletions ...s/babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-classic/plugin.js
View file
Open in desktop
@@ -0,0 +1,13 @@
module.exports = ({ types: t }) => ({
visitor: {
NumericLiteral(path) {
path.replaceWith(
t.jsxElement(
t.jsxOpeningElement(t.jsxIdentifier("p"), []),
t.jsxClosingElement(t.jsxIdentifier("p")),
[]
)
);
}
}
});