Make babel-relay-plugin run before other transforms

[skevy]

babel-relay-plugin depends transforms the “Relay.QL” TaggedTemplateExpression nodes in the source. If it does not run before the “es2015-template-literals” transform, it will not transform the Relay.QL template literals correctly.

This is important, because with Babel 6, you can’t control the plugin order. And so in a case like React Native, where plugins from React Native’s babelrc are loaded before the projects babelrc, it’s impossible to use the Babel Relay Plugin without overriding the entire transform list.

Note - I made this change in the babelAdapter, rather than in the plugin code itself, in order to retain Babel 5 compatibility.

[yungsters]

Why is this required?

In other words, why can't we just use:

path.traverse({TaggedTemplateExpression}, state);

[skevy]

So, the way I understand it is that TaggedTemplateExpression, when in it's normal plugin form, takes in path and state as arguments. state doesn't get passed in, I don't think, when using path.traverse. Instead, state is passed in as this.

Honestly, I'm not a super Babel expert either -- I followed what @thejameskyle did here:

https://phabricator.babeljs.io/rBC0b264aa38af5ba7fb121b216be2fb69eece7217c

[josephsavona]

@facebook-github-bot import

[facebook-github-bot]

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1509715672662867/int_phab to review.

[josephsavona]

@skevy thanks!

[skevy]

@josephsavona woot! one step closer. :)

[josephsavona]

@skevy Since we ended up having to revert due to a conflict with an internal transform, let's reopen this to continue iterating - i'll follow-up hopefully tomorrow with more feedback about direction.

[josephsavona]

@skevy We resolved the conflict with our internal plugin, so we could proceed with reimporting this. However, @DmitrySoshnikov has recently added an option to control plugin ordering in babel. I haven't dug into the React Native OSS setup in a while, but I'm wondering if that same type of configuration would allow the Relay plugin to run before the default RN transforms?

[skevy]

This may not necessarily be needed anymore actually, with some changes that we've done on the RN side (we're going to use a preset now, which plays into taking advantage of @DmitrySoshnikov's changes).

Will test when that gets merged in (should be tomorrow or so) and I'll report back. :-)

Thanks @josephsavona!

[skevy]

@josephsavona so as it turns out...presets still run before things defined in plugins...so really, @DmitrySoshnikov's doesn't help if we want to do this in babelrc:

{
  "presets": ["react-native"],
  "plugins": ["./babelRelayPlugin"]
}

This will still let babel-plugin-transform-es2015-template-literals (which is present in the preset) run before the Relay plugin, and thus still cause breakage.

The only way we'd be able to take advantage of passPerPreset is to actually create a ./babelRelayPlugin preset. I guess you'd make it do something like:

var getBabelRelayPlugin = require('babel-relay-plugin');
var schema = require('../data/schema.json');

module.exports = { plugins: [getBabelRelayPlugin(schema.data)] };

And then do this in your .babelrc:

{
  "presets": ["./babelRelayPlugin", "react-native"],
  "passPerPreset": true
}

It would work...doesn't seem ideal though and seems kind of confusing. Thoughts?

[josephsavona]

@skevy i agree that isn't ideal, but it's not magic and makes it clear to users that the Relay plugin will run first. This means no code changes are required (right?) In that case, would you mind updating the plugin docs to describe how to configure for React Native? :-)

[skevy]

Yah. I suppose it's fine. I can update the docs when this actually becomes possible.

Us non-fb peeps have to wait until this feature is released out into the wild. Gonna check with the Babel folks to get a timeline on when that might be.

[DmitrySoshnikov]

so really, @DmitrySoshnikov's doesn't help if we want to do this in babelrc

Yeah, if you have presets, and passPerPreset, then it should be something like:

passPerPreset: true,
presets: [
  'some-preset',
  {plugins: [ ... ]}, // manual preset
 'other-preset'
]

These presets should run in order. However plugins within each preset are still merged.

[skevy]

When Babel 6.5.0 gets released (which I think is tomorrow?) I'm going to test all this out and I will report back. :)

[josephsavona]

@skevy sounds great, thanks again for following up on this :-)

[skevy]

@josephsavona I can confirm that doing:

{
  "passPerPreset": true,
  "presets": [
    { "plugins": [ "./relay/babelRelayPlugin" ] },
    "react-native"
  ]
}

works as we discussed.

Sometimes, interestingly, this is not necessary, and you can do the more normal:

{
  "presets": ["react-native"],
  "plugins": ["./babelRelayPlugin"]
}

However, I think the fact that it works sometimes is just a fluke in how Babel runs the transforms. Therefore, I think from a documentation perspective, we should recommend passPerPreset (despite it's status as an experimental option, I think it (or some version of it) will stick around)...

[hzoo]

Yeah, I think we would want to change passPerPreset to be a flag in each preset you can set instead of a global one

[DmitrySoshnikov]

Sometimes, interestingly, this is not necessary, and you can do the more normal

Yeah, if you put presets before plugins it seems might run them first, depending on order, which is bad. Anyways, I'd avoid indeterministic behavior, and rely only on fully working solution.

Therefore, I think from a documentation perspective, we should recommend passPerPreset (despite it's status as an experimental option, I think it (or some version of it) will stick around)...

Exactly. I think documenting it is a temporary solution. And then we should just make it a default behavior in Babel: this increases maintainability, stability, and actually make it even working. So for users there is no need to mess with an explicit options, and Babel should just work for them.

[josephsavona]

note that this is a prereq for #26

[pthrasher]

Any update on this?

[josephsavona]

Ok, so the next steps here are:

• Document how to configure the Relay plugin to run before the default React Native plugins
• Work with @DmitrySoshnikov and Babel folks to make it easier to set up plugin/preset ordering.

I'm going to close this PR since we will make a documentation change instead of a code change.