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

Lazily inject imports to the JSX runtime #12493

Merged
merged 3 commits into from Dec 15, 2020
Merged

Lazily inject imports to the JSX runtime #12493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
401b301
Lazily inject imports to the React runtime
nicolo-ribaudo Dec 12, 2020
68ac735
Cleanup
nicolo-ribaudo Dec 12, 2020
e9daba1
Windows
nicolo-ribaudo Dec 12, 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
147 changes: 43 additions & 104 deletions packages/babel-helper-builder-react-jsx-experimental/src/index.js
View file
Open in desktop
Expand Up @@ -43,11 +43,6 @@ export function helper(babel, options) {
const JSX_ANNOTATION_REGEX = /\*?\s*@jsx\s+([^\s]+)/;
const JSX_FRAG_ANNOTATION_REGEX = /\*?\s*@jsxFrag\s+([^\s]+)/;

// This is the number of possible import names
// development: jsxDEV, Fragment, createElement
// production: jsx, jsxs, Fragment, createElement
const IMPORT_NAME_SIZE = options.development ? 3 : 4;

const {
importSource: IMPORT_SOURCE_DEFAULT = DEFAULT.importSource,
runtime: RUNTIME_DEFAULT = DEFAULT.runtime,
Expand Down Expand Up @@ -214,44 +209,33 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
);
}

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

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

state.set(
"@babel/plugin-react-jsx/createElementIdentifier",
createIdentifierParser(
createIdentifierName(path, "createElement", importName),
),
createImportLazily(state, path, "createElement", source),
);

state.set(
"@babel/plugin-react-jsx/jsxFragIdentifier",
createIdentifierParser(
createIdentifierName(path, "Fragment", importName),
),
createImportLazily(state, path, "Fragment", source),
);

state.set(
Expand Down Expand Up @@ -313,44 +297,6 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
return false;
}

function createIdentifierName(path, name, importName) {
if (isModule(path)) {
const identifierName = `${importName[name]}`;
return identifierName;
} else {
return `${importName[name]}.${name}`;
}
}

function getImportNames(parentPath) {
const imports = new Set();

parentPath.traverse({
"JSXElement|JSXFragment"(path) {
if (path.type === "JSXFragment") imports.add("Fragment");
const openingPath = path.get("openingElement");

const validChildren = t.react.buildChildren(openingPath.parent);
let importName;
if (path.type === "JSXElement" && shouldUseCreateElement(path)) {
importName = "createElement";
} else if (options.development) {
importName = "jsxDEV";
} else if (validChildren.length > 1) {
importName = "jsxs";
} else {
importName = "jsx";
}
imports.add(importName);

if (imports.size === IMPORT_NAME_SIZE) {
path.stop();
}
},
});
return imports;
}

function getSource(source, importName) {
switch (importName) {
case "Fragment":
Expand All @@ -367,47 +313,40 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
}
}

function addAutoImports(path, state) {
const imports = getImportNames(path, state);
if (isModule(path)) {
// import {jsx} from "react";
// import {createElement} from "react";
const importMap = {};

imports.forEach(importName => {
if (!importMap[importName]) {
importMap[importName] = addNamed(
path,
importName,
getSource(state.source, importName),
{
importedInterop: "uncompiled",
ensureLiveReference: true,
},
).name;
}
});
function createImportLazily(pass, path, importName, source) {
return () => {
const actualSource = getSource(source, importName);
if (isModule(path)) {
let reference = pass.get(
`@babel/plugin-react-jsx/imports/${importName}`,
);
Comment on lines +320 to +322
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 just wondering - does keeping this in a state has any benefit here? i mean - wouldn't a closure state do the trick just fine? I don't expect any change here - just wondering if there is something I'm not thinking about

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 plugin might be cached, so multiple runs of the plugin over the same file could have the same closure. Thus, we would still need to have a programNode -> imports WeakMap to avoid using imports attatched in a separate Babel run.

if (reference) return t.cloneNode(reference);

return importMap;
} else {
const importMap = {};
const sourceMap = {};
imports.forEach(importName => {
const source = getSource(state.source, importName);
if (!importMap[importName]) {
if (!sourceMap[source]) {
// var _react = require("react")
sourceMap[source] = addNamespace(path, source, {
importedInterop: "uncompiled",
ensureLiveReference: true,
}).name;
}
reference = addNamed(path, importName, actualSource, {
importedInterop: "uncompiled",
Andarist marked this conversation as resolved.
Show resolved Hide resolved
});
pass.set(`@babel/plugin-react-jsx/imports/${importName}`, reference);

importMap[importName] = sourceMap[source];
return reference;
} else {
let reference = pass.get(
`@babel/plugin-react-jsx/requires/${actualSource}`,
);
if (reference) {
reference = t.cloneNode(reference);
} else {
reference = addNamespace(path, actualSource, {
importedInterop: "uncompiled",
});
pass.set(
`@babel/plugin-react-jsx/requires/${actualSource}`,
reference,
);
}
});
return importMap;
}

return t.memberExpression(reference, t.identifier(importName));
}
};
}

function createIdentifierParser(id) {
Expand Down
21 changes: 14 additions & 7 deletions packages/babel-plugin-transform-react-jsx-development/src/index.js
View file
Open in desktop
Expand Up @@ -26,13 +26,20 @@ export default declare((api, options) => {
state.pure =
PURE_ANNOTATION ?? !pass.get("@babel/plugin-react-jsx/pragmaSet");
} else {
state.jsxCallee = pass.get("@babel/plugin-react-jsx/jsxIdentifier")();
state.jsxStaticCallee = pass.get(
"@babel/plugin-react-jsx/jsxStaticIdentifier",
)();
state.createElementCallee = pass.get(
"@babel/plugin-react-jsx/createElementIdentifier",
)();
const getter = get => ({ enumerable: true, configurable: true, get });

// TODO(Babel 8): helper-builder-react-jsx expects those properties to be AST nodes, but we want to
// generate them lazily so that we only inject imports when needed.
// These should actually be functions.
Object.defineProperties(state, {
jsxCallee: getter(pass.get("@babel/plugin-react-jsx/jsxIdentifier")),
jsxStaticCallee: getter(
pass.get("@babel/plugin-react-jsx/jsxStaticIdentifier"),
),
createElementCallee: getter(
pass.get("@babel/plugin-react-jsx/createElementIdentifier"),
),
});

state.pure =
PURE_ANNOTATION ??
Expand Down
2 changes: 1 addition & 1 deletion ...bel-plugin-transform-react-jsx-development/test/fixtures/linux/auto-import-dev/output.mjs
View file
Open in desktop
@@ -1,6 +1,6 @@
import { Fragment as _Fragment } from "react/jsx-dev-runtime";
import { createElement as _createElement } from "react";
import { jsxDEV as _jsxDEV } from "react/jsx-dev-runtime";
import { Fragment as _Fragment } from "react/jsx-dev-runtime";
var _jsxFileName = "<CWD>/packages/babel-plugin-transform-react-jsx-development/test/fixtures/linux/auto-import-dev/input.js";

var x = /*#__PURE__*/_jsxDEV(_Fragment, {
Expand Down
2 changes: 1 addition & 1 deletion ...-transform-react-jsx-development/test/fixtures/windows/auto-import-dev-windows/output.mjs
View file
Open in desktop
@@ -1,6 +1,6 @@
import { Fragment as _Fragment } from "react/jsx-dev-runtime";
import { createElement as _createElement } from "react";
import { jsxDEV as _jsxDEV } from "react/jsx-dev-runtime";
import { Fragment as _Fragment } from "react/jsx-dev-runtime";
var _jsxFileName = "<CWD>\\packages\\babel-plugin-transform-react-jsx-development\\test\\fixtures\\windows\\auto-import-dev-windows\\input.js";

var x = /*#__PURE__*/_jsxDEV(_Fragment, {
Expand Down
21 changes: 14 additions & 7 deletions packages/babel-plugin-transform-react-jsx/src/index.js
View file
Open in desktop
Expand Up @@ -27,13 +27,20 @@ export default declare((api, options) => {
state.pure =
PURE_ANNOTATION ?? !pass.get("@babel/plugin-react-jsx/pragmaSet");
} else {
state.jsxCallee = pass.get("@babel/plugin-react-jsx/jsxIdentifier")();
state.jsxStaticCallee = pass.get(
"@babel/plugin-react-jsx/jsxStaticIdentifier",
)();
state.createElementCallee = pass.get(
"@babel/plugin-react-jsx/createElementIdentifier",
)();
const getter = get => ({ enumerable: true, configurable: true, get });

// TODO(Babel 8): helper-builder-react-jsx expects those properties to be AST nodes, but we want to
// generate them lazily so that we only inject imports when needed.
// These should actually be functions.
Object.defineProperties(state, {
jsxCallee: getter(pass.get("@babel/plugin-react-jsx/jsxIdentifier")),
jsxStaticCallee: getter(
pass.get("@babel/plugin-react-jsx/jsxStaticIdentifier"),
),
createElementCallee: getter(
pass.get("@babel/plugin-react-jsx/createElementIdentifier"),
),
});

state.pure =
PURE_ANNOTATION ??
Expand Down
4 changes: 2 additions & 2 deletions ...nsform-react-jsx/test/fixtures/autoImport/auto-import-react-source-type-module/output.mjs
View file
Open in desktop
@@ -1,7 +1,7 @@
import { createElement as _createElement } from "react";
import { Fragment as _Fragment } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import { createElement as _createElement } from "react";
import { jsx as _jsx } from "react/jsx-runtime";
import { Fragment as _Fragment } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsx(_Fragment, {
children: /*#__PURE__*/_jsxs("div", {
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-plugin-transform-react-jsx/test/fixtures/autoImport/react-defined/output.mjs
View file
Open in desktop
@@ -1,6 +1,6 @@
import { jsxs as _jsxs } from "react/jsx-runtime";
import { createElement as _createElement } from "react";
import { jsx as _jsx } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import * as react from "react";
var y = react.createElement("div", {
foo: 1
Expand Down
2 changes: 1 addition & 1 deletion ...-react-jsx/test/fixtures/react-automatic/concatenates-adjacent-string-literals/output.mjs
View file
Open in desktop
@@ -1,5 +1,5 @@
import { jsx as _jsx } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import { jsx as _jsx } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsxs("div", {
children: ["foo", "bar", "baz", /*#__PURE__*/_jsx("div", {
Expand Down
4 changes: 2 additions & 2 deletions ...orm-react-jsx/test/fixtures/react-automatic/does-not-add-source-self-automatic/output.mjs
View file
Open in desktop
@@ -1,7 +1,7 @@
import { createElement as _createElement } from "react";
import { Fragment as _Fragment } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import { createElement as _createElement } from "react";
import { jsx as _jsx } from "react/jsx-runtime";
import { Fragment as _Fragment } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsx(_Fragment, {
children: /*#__PURE__*/_jsxs("div", {
Expand Down
2 changes: 1 addition & 1 deletion ...abel-plugin-transform-react-jsx/test/fixtures/react-automatic/handle-fragments/output.mjs
View file
Open in desktop
@@ -1,5 +1,5 @@
import { jsx as _jsx } from "react/jsx-runtime";
import { Fragment as _Fragment } from "react/jsx-runtime";
import { jsx as _jsx } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsx(_Fragment, {
children: /*#__PURE__*/_jsx("div", {})
Expand Down
2 changes: 1 addition & 1 deletion ...lugin-transform-react-jsx/test/fixtures/react-automatic/handle-static-children/output.mjs
View file
Open in desktop
@@ -1,5 +1,5 @@
import { jsx as _jsx } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import { jsx as _jsx } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsxs("div", {
children: [/*#__PURE__*/_jsx("span", {}), [/*#__PURE__*/_jsx("span", {}, '0'), /*#__PURE__*/_jsx("span", {}, '1')]]
Expand Down
2 changes: 1 addition & 1 deletion ...jsx/test/fixtures/react-automatic/should-have-correct-comma-in-nested-children/output.mjs
View file
Open in desktop
@@ -1,5 +1,5 @@
import { jsx as _jsx } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import { jsx as _jsx } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsxs("div", {
children: [/*#__PURE__*/_jsx("div", {
Expand Down
2 changes: 1 addition & 1 deletion ...-transform-react-jsx/test/fixtures/react-automatic/should-properly-handle-keys/output.mjs
View file
Open in desktop
@@ -1,5 +1,5 @@
import { jsx as _jsx } from "react/jsx-runtime";
import { jsxs as _jsxs } from "react/jsx-runtime";
import { jsx as _jsx } from "react/jsx-runtime";

var x = /*#__PURE__*/_jsxs("div", {
children: [/*#__PURE__*/_jsx("div", {}, "1"), /*#__PURE__*/_jsx("div", {
Expand Down
4 changes: 3 additions & 1 deletion ...babel-plugin-transform-react-jsx/test/fixtures/regression/issue-12478-automatic/output.js
View file
Open in desktop
@@ -1 +1,3 @@
const foo = /*#__PURE__*/undefined.jsx("p", {});
var _reactJsxRuntime = require("react/jsx-runtime");

const foo = /*#__PURE__*/_reactJsxRuntime.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 fixture shows that it's fixed.