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

fix: smarter handling of user's babel plugins & presets #613

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

justjake
Copy link

@justjake justjake commented May 18, 2020

Motivation

Users should have as much control as possible about the plugins in Linaria's babel pipeline.
Current issues:

Summary

Instead of blindly unshifting PluginItem specifiers to babel's plugins and presets arrays, instead check for the presence of the plugin, and if present, merge existing user-specified plugin options and the options that Linaria requires..

Options are merged like { ...userPluginOptions, ...linariaPluginOptions }, to always prefer the linaria options in case of conflict.

Test plan

I created a reproduction for #612, seen here: https://github.com/justjake/linaria-babel-options-issue-repro

  1. Verify that the build fails (as expected)
  2. Link local checkout of linaria at from this branch. Run yarn build in linaria
  3. Tested with action = extractor -> build now succeeds!
  4. Clean out the cache
  5. Tested with action = shaker -> build now succeeds!

I should also test the new version of Linaria with example integrations in the community, such as the nextjs-linaria example.

This is somewhere between a bug-fix and a new feature. The change has the potential to break existing configs in subtle ways, if user options start being respected. So, maybe consider for inclusion in 2.0.0?

@justjake
Copy link
Author

I believe the test failure is spurious; it seems to be timing-related and I can't reproduce it locally.

@justjake justjake marked this pull request as ready for review May 23, 2020 20:14
@callstack-bot
Copy link

Hey @justjake, thank you for your pull request 🤗.
The coverage report for this branch can be viewed here.

@justjake
Copy link
Author

justjake commented May 26, 2020

I published this branch as https://www.npmjs.com/package/@jitl/linaria for testing purposes.

@turadg
Copy link
Contributor

turadg commented Aug 11, 2020

Thanks for tackling this one @justjake . I'm surprised the only reply yet has been from a bot because I've been stuck on this #612 too.

Sourcing your build library, I got a different error:

console output

Module build failed (from ./node_modules/@jitl/linaria/loader.js):
Error: ~project/src/components/Preview.tsx: An unexpected runtime error ocurred during dependencies evaluation:
Error: Duplicate plugin/preset detected.
If you'd like to use two separate instances of a plugin,
they need separate names, e.g.

  plugins: [
    ['some-plugin', {}],
    ['some-plugin', {}, 'some unique name'],
  ]

Duplicates detected are:
[
  {
    "alias": "~project/node_modules/@jitl/linaria/babel.js",
    "dirname": "~project",
    "ownPass": false,
    "file": {
      "request": "@jitl/linaria/babel",
      "resolved": "~project/node_modules/@jitl/linaria/babel.js"
    }
  },
  {
    "alias": "~project/node_modules/@jitl/linaria/lib/babel/index.js",
    "options": {
      "displayName": false,
      "evaluate": true,
      "rules": [
        {},
        {
          "test": {},
          "action": "ignore"
        }
      ],
      "babelOptions": {
        "sourceType": "unambiguous",
        "presets": [
          [
            "@babel/preset-env",
            {
              "modules": false,
              "loose": true,
              "useBuiltIns": "usage",
              "corejs": 3
            }
          ],
          "@babel/preset-typescript",
          "@babel/preset-react",
          "@jitl/linaria/babel"
        ],
        "overrides": [
          {
            "test": {},
            "compact": true
          }
        ],
        "plugins": [
          "@babel/plugin-transform-flow-strip-types",
          "@babel/plugin-transform-runtime",
          [
            "transform-builtin-extend",
            {
              "globals": [
                "Error",
                "Array"
              ],
              "approximate": true
            }
          ],
          [
            "@babel/plugin-transform-spread",
            {
              "loose": true
            }
          ],
          [
            "@babel/plugin-proposal-class-properties",
            {
              "loose": true
            }
          ],
          "lodash",
          "transform-react-router-optimize"
        ]
      }
    },
    "dirname": "~project",
    "ownPass": false,
    "file": {
      "request": "~project/node_modules/@jitl/linaria/lib/babel/index.js",
      "resolved": "~project/node_modules/@jitl/linaria/lib/babel/index.js"
    }
  }
]
    at assertNoDuplicates (~project/node_modules/@babel/core/lib/config/config-descriptors.js:206:13)

Here's my Babel presets config:

  [
    '@babel/preset-env',
    {
      modules: false,
      loose: true,
      useBuiltIns: 'usage',
      corejs: 3,
    },
  ],
  '@babel/preset-typescript',
  '@babel/preset-react',
  '@jitl/linaria/babel',

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants