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.

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

Allow React automatic runtime to be enforced with an option #18847

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Nov 5, 2020

fixes #18096 (somewhat)

I'm the Emotion maintainer and we've recently released support for the new JSX runtimes that support css prop API in a more automatic way than what has been the default in the past and which is putting a /** @jsx jsx */ into your files. In the past, it could be automated using @emotion/babel-preset-css-prop but tools like CRA doesn't allow u to change Babel config, so you could always resort to the pragma approach (which will still be true with the new transforms - people will just have to use /** @jsxImportSource @emotion/core */ now).

Given that we had this preset it made sense to just add support for the runtime option to it for anyone who has been already using that preset. It also made sense because that preset includes our babel-plugin-emotion so I thought that people could just use a single preset to configure JSX and our Babel plugin at the same time. I also have always thought that plugins are overridden through presets as this is described in the Babel docs, it doesn't go into details and it turns out it doesn't apply to presets but only for "env" configurations. I've reached out to the Babel team and they are going to think about potential changes around this logic but given that it has been out for quite a long time there is near-zero chance for this to be changed in Babel 7. You can check my original expectations which are, unfortunately, failing here.

So what happens now? If you have 2 presets that include the same plugin that plugin will be applied twice and it seems that this is problematic when it comes to the JSX plugin:
#18096
emotion-js/emotion#2064
emotion-js/emotion#2069

So the answer is that one should never configure the JSX plugin twice. In the case of Emotion, it means that we need to deprecate runtime option in our preset and we need to recommend using this simple config:

module.exports = {
  presets: [
    [
      "@babel/preset-react",
      {
        runtime: "automatic",
        importSource: "@emotion/core",
      },
    ],
  ],
  plugins: ["babel-plugin-emotion"],
};

But as Next users very often use next/babel and it uses @babel/preset-react under the hood that recommendation is not good either for Next users and for them it should be this:

module.exports = {
  presets: [
    [
      "next/babel",
      {
        "preset-react": {
          runtime: "automatic",
          importSource: "@emotion/core",
        },
      },
    ],
  ],
  plugins: ["babel-plugin-emotion"],
};

So far so good, but... people usually want to use a single config for all tools that they use. So this doesn't quite work with Jest because hasJsxRuntime depends on the caller object that you configure when creating webpack config etc and thus this very config results in 2 JSX plugins being applied (when used with Jest).

I believe that the easiest way to solve this is what I'm proposing in this PR. If you have any other preferences please let me know and I will adjust the PR accordingly.

@vercel
Copy link

vercel bot commented Nov 5, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/nextjs-examples-image-component/848ikf9qz
✅ Preview: https://nextjs-examples-imag-git-allow-automatic-runtime-to-be-e-994978.vercel.vercel.app

@ijjk
Copy link
Member

ijjk commented Nov 5, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
buildDuration 15.4s 15.3s -114ms
nodeModulesSize 87 MB 87 MB ⚠️ +239 B
Page Load Tests Overall increase ✓
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
/ failed reqs 0 0
/ total time (seconds) 2.967 2.966 0
/ avg req/sec 842.71 842.82 +0.11
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.874 1.85 -0.02
/error-in-render avg req/sec 1334.34 1351.7 +17.36
Client Bundles (main, webpack, commons)
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
677f882d2ed8..7d3b.js gzip 11.3 kB 11.3 kB
framework.HASH.js gzip 39 kB 39 kB
main-5b72373..17dd.js gzip 7.37 kB 7.37 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.4 kB 58.4 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
677f882d2ed8..dule.js gzip 7.04 kB 7.04 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-2d01f96..dule.js gzip 6.37 kB 6.37 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 53.1 kB 53.1 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
polyfills-b5..1119.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_app-7231d4b..5856.js gzip 1.28 kB 1.28 kB
_error-fca3d..2eb1.js gzip 3.44 kB 3.44 kB
hooks-d4591d..e7c2.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-a674d88..ccde.js gzip 1.35 kB 1.35 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.75 kB 7.75 kB
Client Pages Modern
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-a4469f3..dule.js gzip 1.32 kB 1.32 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.41 kB 5.41 kB
Client Build Manifests
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_buildManifest.js gzip 321 B 321 B
_buildManife..dule.js gzip 330 B 330 B
Overall change 651 B 651 B
Rendered Page Sizes
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
index.html gzip 1 kB 1 kB
link.html gzip 1.01 kB 1.01 kB
withRouter.html gzip 996 B 996 B
Overall change 3.01 kB 3.01 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
buildDuration 18.6s 17.9s -680ms
nodeModulesSize 87 MB 87 MB ⚠️ +239 B
Client Bundles (main, webpack, commons)
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
677f882d2ed8..7d3b.js gzip 11.3 kB 11.3 kB
framework.HASH.js gzip 39 kB 39 kB
main-5b72373..17dd.js gzip 7.37 kB 7.37 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.4 kB 58.4 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
677f882d2ed8..dule.js gzip 7.04 kB 7.04 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-2d01f96..dule.js gzip 6.37 kB 6.37 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 53.1 kB 53.1 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
polyfills-b5..1119.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_app-7231d4b..5856.js gzip 1.28 kB 1.28 kB
_error-fca3d..2eb1.js gzip 3.44 kB 3.44 kB
hooks-d4591d..e7c2.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-a674d88..ccde.js gzip 1.35 kB 1.35 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.75 kB 7.75 kB
Client Pages Modern
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-a4469f3..dule.js gzip 1.32 kB 1.32 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.41 kB 5.41 kB
Client Build Manifests
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_buildManifest.js gzip 321 B 321 B
_buildManife..dule.js gzip 330 B 330 B
Overall change 651 B 651 B
Serverless bundles
vercel/next.js canary Andarist/next.js allow-automatic-runtime-to-be-enforced Change
_error.js 1.07 MB 1.07 MB
404.html 4.73 kB 4.73 kB
hooks.html 3.92 kB 3.92 kB
index.js 1.07 MB 1.07 MB
link.js 1.12 MB 1.12 MB
routerDirect.js 1.11 MB 1.11 MB
withRouter.js 1.11 MB 1.11 MB
Overall change 5.49 MB 5.49 MB
Commit: a431804

@Timer Timer closed this in #19136 Nov 13, 2020
@Andarist Andarist deleted the allow-automatic-runtime-to-be-enforced branch November 13, 2020 12:03
@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"React is not defined" when using a custom _app.tsx and .babelrc using React 17
2 participants