Emotion: Emotion Babel Macro duplicating nodes

Created on 15 Jun 2018  路  3Comments  路  Source: emotion-js/emotion

  • emotion version: 9.2.3
  • react version: N/A

Relevant code.

// typography.js
import { css } from 'emotion/macro'

const normal = css`
  font-weight: 400;
`

const demibold = css`
  font-weight: 600;
`

export { normal, demibold }

// compile.js
const fs = require('fs')
const src = fs.readFileSync('typography.js', 'utf-8')
const babel = require('babel-core')
const result = babel.transform(src, {
  filename: '/Users/matt/Projects/sandbox/j2/typo.js',
  plugins: [
    require('babel-plugin-macros'),
    require('@babel/plugin-transform-modules-commonjs'),
  ],
  babelrc: false,
  retainLines: true,
  sourceMaps: 'inline',
})

console.log(result.code)

// <result.code>
"use strict";Object.defineProperty(exports, "__esModule", { value: true });exports.demibold = exports.normal = void 0;var _emotion = require("emotion");

const normal = /*#__PURE__*/(0, _emotion.css)("font-weight:400;");exports.normal = normal;



const demibold = /*#__PURE__*/_css("font-weight:600;");exports.demibold = demibold;

What you did:

Used emotion/macro and babel-plugin-macros to transform code, together with plugin-transform-modules-commonjs

What happened:

The second reference to _css was not replaced with a live reference, causing the output to generate invalid javascript ("_css is not defined" error)

Reproduction:

See above code

Problem description:

Unlike the first reference to the macro import ("css"), the second reference is not replaced with a live reference by the commonjs module transform plugin. This is because when the emotion/macro runs, it is inserting duplicate nodes in the tree - and the module transform plugin is configured to skip nodes it has already visited.

Suggested solution:

Create a new node whenever inserting nodes into the AST, and as an extra verification step, use the checkDuplicateNodes helper:
https://github.com/babel/babel/blob/d383659ca6adec54b6054f77cdaa16da88e8a171/packages/babel-helper-transform-fixture-test-runner/src/index.js#L128

Most helpful comment

I think we just need to clone the returned node here - https://github.com/emotion-js/emotion/blob/0847bd2ba1f2c79beaa5340f8deaeb0ea657e6d6/packages/babel-plugin-emotion/src/babel-utils.js#L69 (with conditional t.clone/t.cloneNode - to keep babel@6 and babel@7 compatibility.

Gonna try to find time to prepare PR for this 2morrow.

All 3 comments

I think we just need to clone the returned node here - https://github.com/emotion-js/emotion/blob/0847bd2ba1f2c79beaa5340f8deaeb0ea657e6d6/packages/babel-plugin-emotion/src/babel-utils.js#L69 (with conditional t.clone/t.cloneNode - to keep babel@6 and babel@7 compatibility.

Gonna try to find time to prepare PR for this 2morrow.

Thanks for the fix @Andarist & also for adding the tests. When do you think the next patch might land on the registry? cheers!

Just did a release

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Aaron-Pool picture Aaron-Pool  路  3Comments

mitchellhamilton picture mitchellhamilton  路  3Comments

eddeee888 picture eddeee888  路  3Comments

hamlim picture hamlim  路  3Comments

Zn4rK picture Zn4rK  路  3Comments