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

Sharing an identifier instance between visitors breaks import aliasing #10377

Closed
kjagiello opened this issue Aug 31, 2019 · 3 comments
Closed
Labels
i: bug i: needs triage outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@kjagiello
Copy link

kjagiello commented Aug 31, 2019

Bug Report

Current Behavior

Sharing an identifier instance between visitors breaks import aliasing.

Input Code

babel-plugin.js

module.exports = function babelPlugin({ types: t }) {
  return {
    name: 'test',
    visitor: {
      Program: {
        enter(path) {
          const program = path.scope.getProgramParent();
         
          // Alias the import so that it doesn't clash with anything else
          // then reuse it when inseting the calls
          this.localName = program.generateUidIdentifier('_foo');

          // Add `import { foo as _foo } from "bar"`
          [this.last] = program.path.unshiftContainer(
            "body",
            t.importDeclaration(
              [t.importSpecifier(this.localName, t.identifier("foo"))],
              t.stringLiteral("bar")
            )
          );
        }
      },
      JSXElement(path) {
        // Add `_foo.doSomething()`
        [this.last] = this.last.insertAfter(
          t.expressionStatement(
            t.callExpression(
              t.memberExpression(this.localName, t.identifier("doSomething")),
              []
            )
          )
        );
      },
    },
  };
};

example.js

import React, { Component } from 'react'
import { Node } from 'some-library'

class App extends Component {
  render() {
    return (
      <>
        <Node uri="test1.txt">hello</Node>
        <Node uri="test2.txt">hello</Node>
        <Node uri="test3.txt">hello</Node>
      </>
    )
  }
}

Expected behavior/code

Transpiled in following way:

BABEL_DISABLE_CACHE=1 babel --verbose example.js  --out-dir dist/

Produces:

// (...)
var _bar = require("bar");
// (...)
_bar.foo.doSomething();
_foo.doSomething();
_foo.doSomething();
// (...)

Expected result:

// (...)
var _bar = require("bar");
// (...)
_bar.foo.doSomething();
_bar.foo.doSomething();
_bar.foo.doSomething();
// (...)

Babel Configuration (.babelrc, package.json, cli command)

babel.config.js

module.exports = {
  env: {
    development: {
      presets: [["next/babel"]],
      plugins: ["./babel-plugin"],
    },
  },
};

Environment

  • Babel version(s): v7.5.5
  • Node/npm version: v12.4.0
  • OS: macOS v10.14.6
  • How you are using Babel: cli

Possible Solution
I'm a complete rookie when it comes to the babel internals, so I'm probably not fit for producing an actual bugfix, but I can share a workaround that I've made for this issue.

I can namely achieve the expected result by recreating the identifier in each visitor instead:

module.exports = function djediBabelPlugin({ types: t }) {
  return {
    name: 'test',
    visitor: {
      Program: {
        enter(path) {
          const program = path.scope.getProgramParent();
         
          // Rename the import so that it doesn't clash with anything else
          // then reuse it when inseting the calls
          this.localName = program.generateUidIdentifier('_foo').name;

          // Add `import { foo as _foo } from "bar"`
          [this.last] = program.path.unshiftContainer(
            "body",
            t.importDeclaration(
              [t.importSpecifier(t.identifier(this.localName), t.identifier("foo"))],
              t.stringLiteral("bar")
            )
          );
        }
      },
      JSXElement(path) {
        // Add `_foo.doSomething()`
        [this.last] = this.last.insertAfter(
          t.expressionStatement(
            t.callExpression(
              t.memberExpression(t.identifier(this.localName), t.identifier("doSomething")),
              []
            )
          )
        );
      },
    },
  };
};
@kjagiello kjagiello changed the title Unexpected behaviour when reusing a generated UID identifier Sharing an identifier instance between visitors breaks import aliasing Aug 31, 2019
@babel-bot
Copy link
Collaborator

Hey @kjagiello! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

@nicolo-ribaudo
Copy link
Member

The workaround you found is the recommended way: a node should never be used twice. It would break all the plugins which only transform each node once. If you don't want to manually recreate the same node, you can use the t.cloneNode method from @babel/types.

You can use https://www.npmjs.com/package/babel-check-duplicated-nodes in your plugin's tests to prevent it from re-using the same node twice.

@kjagiello
Copy link
Author

kjagiello commented Sep 1, 2019

Thanks for the explanation Nicolo. That makes so much sense now that you say it. I'm closing this issue as this isn't a bug in babel, but a bug in the plugin.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 1, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: bug i: needs triage outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

3 participants