chore: migrate rollup-plugin-babel #108

Andarist · 2019-12-23T22:13:57Z

Rollup Plugin Name: `@rollup/plugin-babel`

This PR contains:

Are tests included?

yes (bugfixes and features will not be merged without tests)
no

Breaking Changes?

yes (breaking changes will not be merged unless absolutely necessary)
no

Description

This is still WIP and there is some work left to do regarding migration, but it's ready for a preliminary review. Other than that - I plan to include minor changes to the plugin before it gets released under @rollup scope, so please do not publish this without my green light on that.

I've managed to keep git history from the old repo, which I think is nice here - but it should be merged with squash here.

packages/babel/LICENSE.md

packages/babel/package.json

packages/babel/test/.eslintrc

packages/babel/test/as-input-plugin.js

packages/babel/test/as-output-plugin.js

Andarist · 2019-12-25T20:10:37Z

packages/babel/src/index.js

+  };
+}
+
+const babelPluginFactory = createBabelInputPluginFactory();


I would gladly restructure this - I believe those should be 4 separate exports, rather than a single export with static properties. This PR will land v5 of this plugin so this is IMHO OK to do.

WDYT about those:

default export (result of createBabelInputPluginFactory())

createBabelInputPluginFactory

babelOutputPluginFactory

createBabelOutputPluginFactory

Those named exports would be quite mouthful 🤷‍♂ but correct. I'm open for other names - but cant figure out anything better myself.

IMO the createXY names are ok as they will not be used often and only by people who know what they are doing. However I think functions should start with a verb, and the return value of createBabelOutputPluginFactory should be called getOutputPlugin, which also sounds less intimidating. For symmetry, I would furthermore export the default export additionally as getInputPlugin. This will make @TrySound happy 😃

On second thought getBabelOutputPlugin and getBabelInputPlugin so that it does not conflict with other plugins

Also note that using named and default export together will switch the plugin to named-exports-mode when imported from CJS. This means that const babel = require(‘rollup-plugin-babel’) will no longer work. And I am not sure if the new CLI rollup -c —plugin babel would work. The latter could be fixed by providing a named export babel instead of a default. In any case, this change would be breaking.

I've changed exports to pretty much to what you have suggested, you can check out this commit: 4c5e1a8

remove ignore from preflight options

Andarist · 2019-12-25T21:06:45Z

@NotWoods I've resolved your comments, could you take another look at this?

NotWoods

Tests look pretty good! Some links need updating.

The README and package.json should get updated to match the format of the corresponding files in other plugins. We're trying to use the same template for every plugin in the monorepo.

The CHANGELOG should have an entry for the change in package name and requirements. Other plugins have a similar entry you can copy.

The root README should also be updated with a link to the babel plugin 😄

packages/babel/src/preflightCheck.js

packages/babel/test/as-input-plugin.js

Andarist · 2019-12-27T21:19:26Z

@NotWoods resolved your comments, please take a look especially at the documentation changes.

I still have some work to do, but in the meantime, I'd like to get "partial" approvals on what is already included here.

My TODO list:

docs about babelHelpers option (compile info from Helpers section into it as well) + link to that from the source (there is currently a verbose console.warn there about it)
establish export names - chore: migrate rollup-plugin-babel #108 (comment)
document every change in CHANGELOG

NotWoods

Changelog looks good! README needs a few tweaks to align with the other plugins.

packages/babel/README.md

packages/babel/package.json

shellscape · 2019-12-28T13:51:08Z

@Andarist there seems to be a lot of pushback against conventions that we've set in the repo for migrating this plugin. the conventions we've chosen were to suit the various styles being brought in from repos that were very fragmented. so far, they've worked quite well to bring everything in line and have made maintenance much easier.

If you'd like, I'm happy to go through and update the files that need updating to established conventions so we don't have to engage in the frequent back and forth. I'd like to get you on board, but perhaps it's better to have these discussions after the babel plugin is finished so we can complete the migration? the babel plugin is the last and only outlier, so the likelihood that we'll change the 23 other other plugins we've already completed migration for, before we complete the babel migration is pretty slim.

LarsDenBakker · 2020-03-08T17:05:18Z

What's the status on this? rollup-plugin-babel 5.x is still on an alpha release. This is a vital plugin in the rollup ecosystem.

Andarist · 2020-03-08T17:42:03Z

@LarsDenBakker I'm going to pick up working on this next week. Would appreciate ideas on #108 (comment)

Add repository

Tidy up

…the monorepo

# Conflicts: # pnpm-lock.yaml

Andarist · 2020-04-19T09:43:32Z

@rollup/plugin-maintainers after quite a long time, way longer than I've anticipated 😢, I've got back to it, resolved all comments, updated docs and changelog. This is no longer a draft from my perspective - I would appreciate a review 🙏 so we can move this forward and finally publish it as @rollup/plugin-babel

shellscape · 2020-04-19T11:44:30Z

Thank you, and no worries on the time frame! I'll get to looking at this today!

shellscape · 2020-04-22T15:39:10Z

@Andarist I think we're good on this. I'll fix the pnpm conflict later today/tonight and then we'll merge. 🍻

This avoids fresh `pnpm install` problems where the build runs before babel plugins are installed.

shellscape · 2020-04-26T15:28:30Z

@Andarist no matter what I try locally, I cannot get this to build:

./src/index.js → dist/index.js, dist/index.es.js...
[!] (plugin babel) Error: [BABEL] /Users/powella/code/rollup/plugins/packages/babel/src/index.js: Could not find plugin "proposal-numeric-separator". Ensure there is an entry in ./available-plugins.js for it. (While processing: "/Users/powella/code/rollup/plugins/packages/babel/node_modules/@babel/preset-env/lib/index.js")

Please take a look.

Andarist · 2020-04-26T21:31:22Z

@shellscape fixed

shellscape · 2020-04-26T23:07:13Z

@Andarist I noticed that 5.0.0-alpha.2 was the latest version on rollup-plugin-babel, but the plugin here is only 5.0.0-alpha.1. I also didn't see that you were in an alpha stage, so I'm not sure how to release this because we should release a stable version to archive rollup-plugin-babel.

I guess this is my fault for not looking at the old repo first. Please advise.

Andarist · 2020-04-27T21:36:37Z

Oh, sorry for not mentioning this recently - my intention was this to be a major bump, so just release this as 5.0.0

shellscape · 2020-04-27T23:07:38Z

No worries at all. I'll do that tonight.

enjikaka · 2021-02-10T11:36:41Z

Is this broken again? rollup/rollup-plugin-babel#360

I just tried making two outputs in Rollup, both with a plugin field with babel in them. Getting the same errors as in the issue above;

(!) The "load" hook used by the output plugin babel is a build time hook and will not be run for that plugin. Either this plugin cannot be used as an output plugin, or it should have an option to configure it as an output plugin.
(!) The "options" hook used by the output plugin babel is a build time hook and will not be run for that plugin. Either this plugin cannot be used as an output plugin, or it should have an option to configure it as an output plugin.
(!) The "resolveId" hook used by the output plugin babel is a build time hook and will not be run for that plugin. Either this plugin cannot be used as an output plugin, or it should have an option to configure it as an output plugin.

"@rollup/plugin-babel": "^5.2.3"
"rollup": "^2.38.5"

Andarist · 2021-02-10T11:53:18Z

Is this broken again?

We don't know as you haven't really provided a repro case - there is a non-zero chance that you made some mistake. You could also try to actually debug the problem - the code is all there, on your machine, ready to be investigated.

stevage · 2021-04-15T02:42:39Z

I have made a repro case that shows the same thing: https://replit.com/@stevage/rollup-plugin-repro

Andarist · 2021-04-15T11:14:02Z

@stevage you are using the input plugin in the output plugin's position. Please refer to this doc if you want to use the output plugin: https://github.com/rollup/plugins/blob/master/packages/babel/README.md#running-babel-on-the-generated-code

Andarist commented Dec 23, 2019

View reviewed changes

packages/babel/LICENSE.md Outdated Show resolved Hide resolved

Andarist commented Dec 23, 2019

View reviewed changes

packages/babel/package.json Show resolved Hide resolved

NotWoods requested changes Dec 24, 2019

View reviewed changes

Andarist commented Dec 25, 2019

View reviewed changes

Andarist force-pushed the babel branch from 64c9181 to 58480ce Compare December 25, 2019 20:54

Andarist pushed a commit that referenced this pull request Dec 25, 2019

Merge pull request #108 from rollup/gh-102

36135d1

remove ignore from preflight options

NotWoods requested changes Dec 25, 2019

View reviewed changes

packages/babel/src/preflightCheck.js Outdated Show resolved Hide resolved

packages/babel/src/preflightCheck.js Outdated Show resolved Hide resolved

packages/babel/test/as-input-plugin.js Outdated Show resolved Hide resolved

packages/babel/test/as-input-plugin.js Show resolved Hide resolved

NotWoods requested changes Dec 27, 2019

View reviewed changes

shellscape reviewed Dec 28, 2019

View reviewed changes

packages/babel/package.json Show resolved Hide resolved

shellscape reviewed Dec 28, 2019

View reviewed changes

packages/babel/package.json Outdated Show resolved Hide resolved

TrySound mentioned this pull request Jan 21, 2020

rename package name to match @rollup npm scope rollup/rollup-plugin-babel#347

Closed

Andarist mentioned this pull request Jan 22, 2020

Problem with ESM import syntax with pluginutils rollup/rollup-plugin-babel#348

Closed

shellscape force-pushed the master branch 2 times, most recently from cd63d3c to 98990a2 Compare January 25, 2020 01:56

Andarist mentioned this pull request Apr 9, 2020

Run babel per output rollup/rollup-plugin-babel#333

Merged

Rich-Harris added 10 commits April 11, 2020 18:45

initial commit

8df673c

allow include/exclude

560685a

admin and README

7d8ad81

-> 1.0.0

6aa95af

fix prepublish script

f5d686c

Update README.md

f037057

make compatible with babel 6

e07ef8d

-> 2.0.0

c1dbeb4

Merge pull request #6 from Victorystick/patch-1

fcfab7b

Add repository

Merge pull request #8 from TrySound/tidy-up

360adcb

Tidy up

Andarist added 6 commits April 19, 2020 00:04

Align package.json of Babel plugin to existing package.json files in …

3f6f06b

…the monorepo

Update @rollup/pluginutils in the Babel plugin

91959c7

Merge branch 'master' into babel

c2b3401

# Conflicts: # pnpm-lock.yaml

Add new changelog entries to the Babel plugin

9ee8dff

Move babelHelpers recommendation text to the README

3108a6d

Fix typo in the README

dcf694c

Andarist marked this pull request as ready for review April 19, 2020 09:40

Andarist mentioned this pull request Apr 19, 2020

Make this output plugin rollup/rollup-plugin-babel#360

Closed

shellscape requested a review from NotWoods April 22, 2020 15:38

LarsDenBakker mentioned this pull request Apr 23, 2020

bundled helpers when using .generated #331

Closed

shellscape added 2 commits April 26, 2020 11:21

chore: Merge branch 'master' into babel

e62a373

chore: remove prepare script, build in prepublishOnly

516d789

This avoids fresh `pnpm install` problems where the build runs before babel plugins are installed.

Upgrade @babel/preset-env in babel plugin

dcc1182

shellscape merged commit 440dbc4 into master Apr 26, 2020

shellscape deleted the babel branch April 26, 2020 21:53

shellscape mentioned this pull request Apr 27, 2020

ERROR 404 Not Found: @rollup/plugin-babel #338

Closed

stevage mentioned this pull request Apr 15, 2021

Not working as an output plugin (warnings printed) #858

Closed

Uh oh!

chore: migrate rollup-plugin-babel #108

chore: migrate rollup-plugin-babel #108

Uh oh!

Conversation

Andarist commented Dec 23, 2019

Rollup Plugin Name: @rollup/plugin-babel

Description

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Andarist Dec 25, 2019

Choose a reason for hiding this comment

Uh oh!

lukastaegert Mar 8, 2020

Choose a reason for hiding this comment

Uh oh!

lukastaegert Mar 8, 2020

Choose a reason for hiding this comment

Uh oh!

lukastaegert Mar 8, 2020

Choose a reason for hiding this comment

Uh oh!

Andarist Apr 11, 2020

Choose a reason for hiding this comment

Uh oh!

Andarist commented Dec 25, 2019

Uh oh!

NotWoods left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Andarist commented Dec 27, 2019

Uh oh!

NotWoods left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shellscape commented Dec 28, 2019

Uh oh!

LarsDenBakker commented Mar 8, 2020

Uh oh!

Andarist commented Mar 8, 2020

Uh oh!

Andarist commented Apr 19, 2020

Uh oh!

shellscape commented Apr 19, 2020

Uh oh!

shellscape commented Apr 22, 2020

Uh oh!

shellscape commented Apr 26, 2020

Uh oh!

Andarist commented Apr 26, 2020

Uh oh!

shellscape commented Apr 26, 2020

Uh oh!

Andarist commented Apr 27, 2020

Rollup Plugin Name: `@rollup/plugin-babel`

enjikaka commented Feb 10, 2021 •

edited

Loading