Add support for sequenceExpressions

[baer]

👋 Hi @ericf - I just ran into this problem in my babel-plugin and see that you have the same.

Babel's transform of import uses sequence expressions in it's output. Also, tools like isparta, adana and istanbul use these to instrument the code. In any case, this is the result of one of these transformations:

var _reactIntl = require("react-intl");

var defaultMessages = (0, _reactIntl.defineMessages)({
  'greeting-button': {
    id: 'greeting-button',
    description: 'The button greeting developers on the front page of the getting started app',
    defaultMessage: 'Hi! {name} {environment}'
  }
});

At the moment the referencesImport function you have only checks for identifiers. This PR extends it to also handle the above case. Here is an AST snippit demoing the issue and the fix: http://astexplorer.net/#/NdbZIrInxo/4

Side note, I have a test for this case in my plugin but I see you don't have any tests yet. I wrote a testing tool for babel plugins that just asserts a before and after if you wanna use it. I can help you write the first few tests if you'd like:

Here's the link
https://github.com/walmartreact/assert-transform

And a lib using it:
https://github.com/walmartreact/babel-plugin-i18n-id-hashing

[baer]

Now that I've thought a bit about it, that testing tool won't be very helpful to you since it's testing a before and after of a transform. Oh well.

[ericf]

This sounds like it might be a bug in Babel. I haven't seen this issue before, how are you hitting it?

[baer]

Humm, I just tested this in ASTexplorer and it must be a babel plugin that is doing this. Likely it's the isparta/istanbul instrumentation. I'm looking now to find where it comes from. Regardless, it is valid JS and should be handled if possible IMO.

I hit this when I found that our coverage reporter was running babel twice. Once for instrumentation (which ran the plugins defined in the .babelrc) and another time in Karma via the babel-loader. This is a bug in isparta/istanbul which I hope to sort out this week too.

[baer]

Update - this is the standard output from compiling an import:

import {defineMessages} from "react-intl";

var defaultMessages = defineMessages({
  'greeting-button': {
    id: 'greeting-button',
    description: 'The button greeting developers on the front page of the getting started app',
    defaultMessage: 'Hi! {name} {environment}'
  }
});

Compiles To

'use strict';

var _reactIntl = require('react-intl');

var defaultMessages = (0, _reactIntl.defineMessages)({
  'greeting-button': {
    id: 'greeting-button',
    description: 'The button greeting developers on the front page of the getting started app',
    defaultMessage: 'Hi! {name} {environment}'
  }
});

http://astexplorer.net/#/8AW2vv5pBp

[ericf]

Regardless, it is valid JS and should be handled if possible IMO.

A major design of this plugin was to not work with any possible valid JS, but restrict to code which can be statically analyzed with high assurance, which is why I limited to only checked ES6 module import syntax and not work with require(), etc.

[baer]

Humm, that's true. So far the only use cases I've found for this bit of JavaScript arcana are:

1. Compiling import statements
2. Instrumenting code

You have very clearly not supported the first case and that has kept the code very tight but the but he 2nd seems likely to come up again. If you were to use a babel-plugin for instrumentation, plugin ordering could cause react-intl to break.

That said, I do not have any examples of plugins beyond test coverage (istanbul, isparta, adona etc.) that do this and since it's very unlikely that somebody will use a CI run to generate the language bundle for prd this seems unlikely.

I respect that you want to keep the scope and size of this thing limited. Since my plugin does actual transformation of the code, this bug bit me bad since it broke all of my tests and was relatively deep.. For this plugin, ¯_(ツ)_/¯. Feel free to close if you're not interested in supporting this case.

👋 over and out 👋

[ericf]

Since my plugin does actual transformation of the code, this bug bit me bad since it broke all of my tests and was relatively deep

Yeah, people were running into this issue when I had this plugin transforming the code because of the same underlying problem: Babel was running twice and the transformations weren't idempotent. With Babel 6 and the way it merges configs, it's very easy to have plugins run multiple times without knowing it. The way I fixed this was to make this plugin idempotent (#28).

If I'm understanding you correctly, then this plugin should work fine in the case you mentioned where tooling is running Babel twice because the first time it will extract the message descriptors and the second time it will noop.

(I do still want to pull in your other PR though… I can work on integrating it.)

[baer]

No need to work on integrating it if it's not useful. I'll see if I can give a hand on some of the leftover tasks in react-intl proper.

[ericf]

Yeah, #20 is something I want to pull in. I'll work on it this week, it just has merge conflicts.

[baer]

Ah, if you're still interested in that PR I can rebase it and solve them for ya. No need to have you fixing my code :)