Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

Support of babel 6 and babel 7 #14

Open
igor-dv opened this issue Sep 3, 2018 · 6 comments
Open

Support of babel 6 and babel 7 #14

igor-dv opened this issue Sep 3, 2018 · 6 comments

Comments

@igor-dv
Copy link

igor-dv commented Sep 3, 2018

Hey,

I would like to use this package in Storybook. In my PR here, I've temporarily copied the code in order to make a progress.

The problem is that Sotrybook has a peer of bable-loader in order to allow people still to use babel 6.
That means, we can't be dependant on a package that is directly dependant on @babel/core.
So I've created a babel-core-proxy that does something like this:

/* eslint-disable import/no-extraneous-dependencies,global-require */
let babelCore = null;

try {
  babelCore = require('@babel/core');
} catch (e) {
  babelCore = require('babel-core');
}

module.exports = babelCore;

And then used it in the babel-merge code:

import { resolvePlugin, resolvePreset } from './babel-core-proxy';

Will you consider this change as a pull request ?

The downside of this approach is that babel-core and @babel/core can't be in a list of peerDeps, but it's something that can be documented 🤷‍♂️

Thanks.

@ndelangen
Copy link

Hey @neutrinojs we'd really love to use this package over at storybook, do you think if @igor-dv made a PR you'd accept it?

@edmorley
Copy link
Member

edmorley commented Sep 4, 2018

Hi! Thank you for reaching out :-)

In the Storybook PR, I see references to Babel 7 specific package names, eg:
https://github.com/storybooks/storybook/pull/4027/files#diff-6b922c89c3c27503a2a2eb1dc0112578R5

It seems like even with the proposed changes here, that won't work under Babel 6?

The downside of this approach is that babel-core and @babel/core can't be in a list of peerDeps, but it's something that can be documented 🤷‍♂️

Do you mean deps instead of peerDeps? ie: Users of babel-merge would need to follow the documentation since there will no no dep or peeDep on babel core? If so, perhaps I think there is a way for babel-merge to be able to have a peerDep on babel core - using the bridge package:
https://github.com/babel/babel-bridge

Babel 7 has so many improvements, it seems like encouraging users to upgrade to it would be worth the effort long-term. There's a Babel package for assisting with migration that to which users could be directed?
https://github.com/babel/babel-upgrade

However @eliperelman has more of a stake in the direction of this package than I, so will probably have more thoughts :-)

@edmorley
Copy link
Member

edmorley commented Sep 4, 2018

Hmm actually thinking about this, I believe that babel-merge should never have had a dependency on @babel/core in the first place - it should always have been in peerDependencies. As such, the bridge package seems like the correct solution.

@igor-dv
Copy link
Author

igor-dv commented Sep 4, 2018

In the Storybook PR, I see references to Babel 7 specific package names, eg:

it's true, but people that still use babel 6, can just use their .babelrc, instead of ours, and it will work for them.

Users of babel-merge would need to follow the documentation since there will no dep or peeDep on babel core

Yeah 😅

babel-core

It's actually a nice idea.

peerDependencies: {
  "babel-core": "6.x || ^7.0.0-bridge.0"
}

Again, there will be a need to install the bridge even if you are using babel@7. Also, we will need to have a peer on "babel-core": "6.x || ^7.0.0-bridge.0" 🤔

@ndelangen
Copy link

Sounds awesome @edmorley

@eliperelman
Copy link
Member

I'm totally fine going the bridge peerDep route. Worst case scenario, we can backport some of the merge changes from 3.x into 2.x which works on Babel 6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants