Use the peerDep to load types/template/traverse in plugins

[loganfsmyth]

Q	A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR
Any Dependency Changes?
License	MIT

We've got it, so I'd say we should use it.

I haven't done it in this PR, but one big benefit here is that we can hoist more of our transform utilities into the top-level module scope to make the core of our plugin functions smaller, which should also help make them more readable.

The other reason I'm adding this is that I'd like to use a helper module for caching, and it'll require injecting code into each plugin. I could explicitly add a dependency to every plugin for that helper, but it's just as easy to expose that via babel-core and then not have to add dependencies to the plugins at all.

Thoughts?

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5709/

[hzoo]

sounds like a why not to me. Yeah I agree if we are going to go ahead with peerDeps since makes sense. Less deps in the plugins themselves and also guarantees it's the same version.

I wonder why a plugin needs api now haha, do we even have any plugins that would need it if you have a peerDep?

[loganfsmyth]

I wonder why a plugin needs api now haha, do we even have any plugins that would need it if you have a peerDep?

You're not wrong. I'm for leaving them for now for backward-compatibility for Babel 6, but we could definitely consider making api a much simpler API in the future by not exposing the Babel object, instead just showing the APIs related to plugins, like we've got api.env() to get the name of the current BABEL_NODE || NODE_ENV, and api.cache for more general caching. It'd be nice if it was just those.

[benjamn]

Is there still time to consider reverting this change?

For Meteor developers, this means anyone using custom .babelrc plugins must now install @babel/core in their application node_modules directory, instead of relying on the plugin API object. The @babel/core package and its dependencies take up 9.2MB of unnecessary disk space, but more importantly the API object was guaranteed to be consistent with the version of Babel that Meteor uses to run the plugins, and this PR sacrifices that consistency.

See meteor/meteor#9554 (comment) for an example. You might think that the peer dependency on @babel/core would at least cause a warning when the developer runs (for example) npm install @babel/plugin-proposal-optional-chaining, but npm@5.6.0 apparently doesn't enforce peer dependencies, not even with a warning!

% npm --version        
5.6.0
% npm install @babel/plugin-proposal-optional-chaining
+ @babel/plugin-proposal-optional-chaining@7.0.0-beta.38
added 2 packages in 1.078s

The only time I've seen a warning about the missing peer dependency is when I run npm ls after installing a package. I think this poses a problem for your claim that "We've got it, so I'd say we should use it."

From what you've said above, you don't seem super convinced that this change was necessary (just that it was feasible and seemed safe), so I hope I've demonstrated a use case that will be impaired by this change. The wisdom of this change depends on the enforcement of peer dependency constraints, so it seems like a pretty serious problem that peer dependencies are not reliably enforced by npm.

[nicolo-ribaudo]

I don't know how meteor works, but doesn't it depend on @babel/core? npm/yarn should dedupe it.

[benjamn]

Yes, of course Meteor uses @babel/core, but we don't require it to be installed in the application's node_modules directory.

Meteor calls babel.transform with a set of plugins and presets that have been imported from a package called meteor-babel, so Babel doesn't have to import those packages itself every time it transforms a file. This solves a lot of problems for us, since we can reliably shrinkwrap the exact versions of all those packages, without conflicting with whatever the developer has installed manually.

Developers who want to use additional plugins can do so by installing them in their application's node_modules directory and then mentioning them in a custom .babelrc file. This PR makes that part harder, since peer dependencies are apparently not something that npm actually enforces. Meteor developers can npm install @babel/core to fix this, but (1) they don't get any warning from npm that they should do that, and (2) getting the .types API from the plugin API object was working just fine before this PR.

[loganfsmyth]

Well this got long haha. I'm happy to discuss it further.

The primary reason for this approach, I'd say, is that separating control of @babel/core version and .babelrc/plugin versions encourages version mismatches that are generally hard and/or impossible for users to diagnose and resolve.

I didn't realize Meteor allowed users to customize via .babelrc. The way separating .babelrc from core in Meteor would worry me is, what happens now, for instance, if we make a breaking change to some plugin so it doesn't work on older versions of @babel/core's beta? The user will already have the old core version installed, but if they want to pick up a new version of a plugin from us, they may start getting random exceptions because the one Meteor installed is too old.

Alternatively, it looks like https://github.com/meteor/babel/blob/master/package.json#L32 uses ^. What happens if a user installs @babel/preset-env@7.0.0-beta.38 as an exact version, and tomorrow we make a breaking change to @babel/core (since it's still in Beta) that breaks compatibility with beta.38? When they reinstall their node_modules they'd get a new @babel/core@beta.39 via Meteor, but their plugin with be beta.38 and break. Obviously once we're out of beta that case will be less common, at least.

The problem we're attempting to address here is that if a user has a .babelrc, that means they've installed plugins, and essentially 100% of the time those plugins are going to be the core ones like preset-env. Those plugins are going to be 1-1 with a version of babel-core and when used on the wrong version, they'll probably throw random errors.

Compounding this, Babel 6 was around for a long time, and development of 7 has been slow because we took on too much. We'd really like to be releasing major versions much more often going forward. If every framework has a direct dependency on Babel, it's going to be extremely painful for users using more than one piece of tooling to upgrade because they'd have to upgrade all of their tooling to a new Babel version in sync to keep matching core versions.

It may be trivial in the single-case, but take for example a user using Meteor, with Jest to run tests, what happens when Meteor is using Babel 7 and Jest has updated to Babel 8? The user now has a .babelrc that can only ever work on one of those versions, so they are trapped. By making @babel/core a peerDependency, we encourage frameworks to be more agnostic to specific versions of @babel/core by separating that dependency out so the user can keep their Babel version matched up with the versions of their plugins.

That has led us to aiming more for a peerDependency approach to allow packages using Babel to depend more broadly via say a peer dependency on 7 | 8 | 9 and so on.

Given all that, the thought has been on a world where either:

1. User installs a main package that depends directly on @babel/core, in which case that package should be the one loading all plugins, and should pass babelrc: false.
2. User installs a package that doesn't know specifically about Babel, but can install an integration to opt in, and that integration would include a peerDependency, and allow the user to use .babelrc, e.g. npm install meteor meteor-babel @babel/core

Curious to hear your thoughts on alternative ways to address this. I'll happily admit that it's has downsides.

[benjamn]

Meteor provides a consistent, well-tested core of Babel plugins and presets that are known to work well with each other, much like preset-es2015 or preset-env. Specifically, we maintain our own legacy and modern presets, plus some additional plugins depending on the environment.

We've been clear with our developers that .babelrc is a power-user feature for any extra plugins that they might need/want to add, and we expect most developers will never need to manage their own plugins. If you don't have a .babelrc file, Meteor compiles your code using meteor-babel, and everything just works. That's how we maintain consistency between utility packages like @babel/core and the plugins that we've curated.

This abstraction has made it vastly easier to update Meteor from Babel 5 to 6 and now 7, because everything just works if you don't have a .babelrc file, and it's up to power users to verify their .babelrc files are up-to-date when they decide to update to a version of Meteor that uses a different major version of Babel.

Can you tell me if the following statement is accurate? If a Meteor power-user decides to use a custom Babel plugin via .babelrc (in addition to the core plugins that Meteor provides), she is better off using her own version of @babel/core than relying on the API object that Meteor passes to her custom plugin.

If that statement sounds reasonable to you, then I suppose forcing users to install @babel/core is reasonable, given that they're taking matters into their own hands already by having a .babelrc file.

[benjamn]

We've been having a detailed discussion on the BabelJS #development slack channel: https://babeljs.slack.com/archives/C062RC35M/p1516393104000400