-
-
Notifications
You must be signed in to change notification settings - Fork 599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: migrate rollup-plugin-babel #108
Conversation
packages/babel/src/index.js
Outdated
}; | ||
} | ||
|
||
const babelPluginFactory = createBabelInputPluginFactory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought getBabelOutputPlugin
and getBabelInputPlugin
so that it does not conflict with other plugins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed exports to pretty much to what you have suggested, you can check out this commit: 4c5e1a8
remove ignore from preflight options
@NotWoods I've resolved your comments, could you take another look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😄
@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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog looks good! README needs a few tweaks to align with the other plugins.
@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. |
cd63d3c
to
98990a2
Compare
What's the status on this? |
@LarsDenBakker I'm going to pick up working on this next week. Would appreciate ideas on #108 (comment) |
Add repository
# Conflicts: # pnpm-lock.yaml
@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 |
Thank you, and no worries on the time frame! I'll get to looking at this today! |
@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.
@Andarist no matter what I try locally, I cannot get this to build:
Please take a look. |
@shellscape fixed |
@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. |
Oh, sorry for not mentioning this recently - my intention was this to be a major bump, so just release this as 5.0.0 |
No worries at all. I'll do that tonight. |
* initial commit * allow include/exclude * admin and README * -> 1.0.0 * fix prepublish script * Update README.md * make compatible with babel 6 * -> 2.0.0 * Merge pull request rollup#6 from Victorystick/patch-1 Add repository * Merge pull request rollup#8 from TrySound/tidy-up Tidy up * -> 2.0.1 * remove unnecessary logging * bump up timeout, to try and fix tests on 0.12 * generate sourcemaps by default * -> 2.1.0 * test for rollup#16 * run preflight check per file – fixes rollup#16 * -> 2.2.0 * Merge branch 'master' of https://github.com/rollup/rollup-plugin-babel * debug travis test failure * add missing devDep * add appveyor config * es6ify source code * fix tests on windows? * install babel-register * Merge pull request rollup#19 from rollup/rollupgh-17 Better handling of helpers * -> 2.3.0 * -> 2.3.2 * fix incomprehensible travis test failure * fix helper renaming (rollup#22) * -> 2.3.3 * fix failing preflight check in absence of class transformer (closes rollup#23) * -> 2.3.4 * Merge branch 'master' of https://github.com/rollup/rollup-plugin-babel * use class transformer local to plugin * -> 2.3.5 * typo * verify warnings are correct * prevent unnecessary `var babelHelpers = {}` * update rollup * Merge pull request rollup#29 from rollup/fix-preflight-check-caching Ensure caching works properly for repeated calls with the same `dir`. * -> 2.3.6 * remove some debugging code * Merge pull request rollup#31 from ericf/fix-babel-helper-bindings Fix Babel helper binding renaming * -> 2.3.7 * -> 2.3.8 * external-helpers-2 -> external-helpers * Merge remote-tracking branch 'ericf/remove-babel-helper-renaming' * -> 2.3.9 * Update README.md * Merge pull request rollup#41 from joeldenning/external-helpers Adding support for referencing external, global helpers * -> 2.4.0 * import helpers, so that transform function is pure * update test * update deps, switch from babel to buble * -> v2.5.0 * install buble * Merge pull request rollup#57 from rollup/mocha-opts Extract mocha options into their own file. * Merge pull request rollup#56 from rollup/rollupgh-55 typeof * -> v2.5.1 * Update dependencies. * dont export _typeof, _extends etc * use NUL convention for helper name * -> v2.6.0 * -> v2.6.1 * typo * Merge pull request rollup#73 from zeitiger/updateDocumentationForBabel63 update README for new Babel 6.13 * update dependencies * Merge branch 'issue/81-generates-sourcemap-by-default-spec-failing' of https://github.com/snuggs/rollup-plugin-babel into snuggs-issue/81-generates-sourcemap-by-default-spec-failing * update build matrix * update appveyor matrix * update babel-preset-es2015-rollup for tests * test with babel-preset-es2015-rollup@3 * Merge pull request rollup#90 from DrewML/update-babel-warning Remove recommendation to change Babel preset from lib * Merge pull request rollup#85 from rollup/use-onwarn-if-available Use options.onwarn if available * Merge pull request rollup#94 from rtsao/patch-1 Updated external-helpers package name in README * Merge pull request rollup#99 from AMcBain/master Ignore Babel 'only' options property for preflight checks. * Merge pull request rollup#92 from sorgloomer/master Added externalHelpersWhitelist option * -> v2.7.0 * Merge pull request rollup#108 from rollup/rollupgh-102 remove ignore from preflight options * Merge pull request rollup#107 from rollup/rollupgh-105 fix duplicated helper warnings with runtime helpers * Merge pull request rollup#109 from rollup/rollupgh-95 Allow custom runtime-helpers moduleName * -> v2.7.1 * -> v2.7.1 * fix prepublish script * Merge pull request rollup#117 from vinkla/patch-1 Simplify repository config * make use of object-assign (rollup#118) Delete assign from utils.js * Breaking: Babel 7 alpha update (rollup#139) Breaking: Babel 7 alpha update * 3.0.0-alpha.12 * update to babel 7 alpha.15 * 3.0.0-alpha.15 * use alpha.17 (rollup#151) * 3.0.0-alpha.17 * Peer dep (rollup#153) * make an inline plugin for the preflight check, make babel-core a peerDep * turn babel-core dep into a peerDep * fix reference to old class plugin * update lock file, ignore log files * 3.0.0 * 3.0.0 changelog [skip ci] * fix transform (rollup#154) * 3.0.1 * changelog [skip ci] * Fix regression with Babel 6 (rollup#158) * Fix regression with Babel 6 Since in Babel 6 it is not supported to replace the argument of a default export declaration with an expression, use a custom "replaceWith" function which handles also that case. Fixes rollup#155, fixes rollup#156. * Run tests using both Babel 7 and Babel 6 * 3.0.2 * Fixed support for babel 7 (rollup#170) * Fixed support for babel 7 * feat(babel): Use scoped babel packages BREAKING CHANGE: This now requires scoped babel packages to be used. * Updated to @babel/plugin-proposal-decorators * Replace es2015 preset with env (rollup#174) * Merge pull request rollup#150 from wmhilton/patch-1 Use "module" in addition to "jsnext:main" * 4.0.0-beta.0 * Merge pull request rollup#175 from rollup/existentialism-patch-1 Fix small grammar nit in external-helpers warning * Update README.md (rollup#163) Added a missing word and fixed few tiny errors * Update rollup dependencies and fix tests and build * merge master -> update-deps * Merge pull request rollup#147 from kopax/master clarify README.md showing an example excluding helpers with a variable set to true * Refactored async tests to bare promises to fix them on older nodes (rollup#194) * Fixed issue with reading .code from null (untranspiled file due to ignore/only options) (rollup#193) * 4.0.0-beta.2 * Restore NUL convention for helper name (rollup#198) The null character `\0` seems to have first been introduced by d169da1. Fixes rollup#197 * 4.0.0-beta.3 * bumped babel deps * 4.0.0-beta.4 * Fix npm beta label in README (rollup#207) Previously, the instructions for usage with Babel 7 was to install the plugin with the label "next" which doesn't exist. The correct label is "beta". * Bumped version of babel deps * Tweaked preflight check (fixes rollup#211) * 4.0.0-beta.5 * Drop node@4 support * Bumped all dependencies. (rollup#219) * Update pinned version of babel. * Update all development dependencies so this plugin is tested against current software. * Make changes to test needed by updated rollup. * Add node.js 10 to testing. * Delegate warnings and errors reporting to rollup (rollup#223) * Delegate warnings and errors reporting to rollup * Fixed mocha deprecation warn * Merge branch 'option-cleanup' * Cache preflight check results per plugin's instance (rollup#224) * Merge remote-tracking branch 'miketrexler/modules-false-docs' * Tweaked babelrc: false workaround recommendation * Throw when used with module transform BEFORE throwing about unexpected situation (rollup#225) * Updated deps: eslint, rollup, pluginutils - tests still green * Fix for issue rollup#220: Check ESM first before checking inheritance helper + Expect imported package instead of variable name which feels safer for checking runtime usage. * Fine-tuned runtime check to allow custom helper name. Issue rollup#220. * Added new previously failing test for issue rollup#220. * Made hints related to babel modules:false more explicit and helpful. * Extracted error messages into variables, splitted by lines for better code readability. Template literals are not allowed right now by linting. * Removed irrelevant babel plugin from a test case * 4.0.0-beta.6 * Fixed runtime helpers detection with useESModules & useBuiltIns options (rollup#226) * Add support for esm and builtin runtime helpers This diff adds support for `useESModules` and `useBuiltIns` options in transform-runtime plugin which injects paths with `es6` and `builtin` suffixes. * Fix lint * 4.0.0-beta.7 * Improve distributed dist directory (rollup#234) * Improve dist - rename .es.js to .esm.js - removed jsnext:main - simplified rollup config * Tweak files and revert external * Upgrade babel deps and tweak preflight check according to latest changes (rollup#233) * Upgrade babel and tweak according changes - simplified preflight check for runtime helpers - removed moduleName test - tweaked removed options * Fix lint * Regenerate lockfile * 4.0.0-beta.8 * Transpile only default babel extensions, allow for custom extensions option (fixes rollup#111) * Pass appropriate caller config to babel.transform (rollup#238) * 4.0.0 * Update docs regarding babel versions and matching plugin versions * Bumped babel deps * 4.0.1 * Fix `rollup` peer dependency (rollup#240) Semver range `^0.60.0` doesn't include `0.65.0`, etc., as if major version is 0, minor version updates indicate breaking changes. `>1` is added for future-proof. * 4.0.2 * Tweaked README, added 4.0 CHANGELOG notes * Fixed fallbackClassTransform * 4.0.3-0 * 4.0.3 * Fix preflightChecks for ignored files * Setup prettier * Run prettier over the repo files * Tweak eslint rules to stop conflicting with prettier output * 4.1.0-0 * Add options.extensions to README (rollup#256) * Upgraded husky * Remove babel 6 test from AppVeyor (rollup#263) * Remove babel 6 test from appveyor * Remove reduntant echo * Fix indentation in examples of configuration in readme (rollup#266) * Return null from the transform hook when it doesn't transform anything * 4.1.0-1 * 4.1.0 * Reword 'except' as 'plus' (rollup#280) * Allow rollup@1 as peer dep * 4.2.0 * Update devDep to rollup@1 * Fail build when trying plugin tries to add non-existent helper (rollup#290) * Merge branch 'custom-builder' * 4.3.0 * Fix reference to the plugin in the README * Update CHANGELOG * Add js extention to helper file (rollup#296) * 4.3.1 * Add note to CHANGELOG about v4.3.1 * Fix usage with externalHelpers flag * 4.3.2 * Update CHANGELOG with note about 4.3.2 * chore: audit fixes * chore: npm audit fix * Update rollup-pluginutils@2.8.1 (rollup#311) * Update rollup-pluginutils@2.8.1 Update `rollup-pluginutils` to a version which contains a fix for the `braces` package vulnerability. https://nodesecurity.io/advisories/786 * Regenerate package-lock * 4.3.3 * Add possibility to run babel per output (rollup#333) * Update dependencies * First draft for using the plugin to transform output options * Deduplicate code * Pass correct context to transformCode * Update documentation and add chunking test * Fix Windows tests * Address review comments * No longer respect "file" and "dir" options and adjust docs * Use plugin.generated for output plugin * Add warning for formats other than ESM or CJS, update docs * Throw for not recommended formats but add an override option. * Address review comments * Fixed issue with incorrect caller data being passed to Babel by generated output plugins * Reduce information about external helpers as those are not recommended * Add `sourceType` when transforming the output. * Update test and readme for runtime helpers * Add note about babelrc files * Do not load config files by default * Update dependencies * Note minimal Rollup version for output plugins * Merge branch 'tla' # Conflicts: # src/index.js * Use babel.transformAsync (rollup#331) * Use rimraf in prebuild script (rollup#335) * use rimraf * move to dev dep * Drop node 6 support, use untranspiled async/await syntax * Recommend explicit babelHelpers option to be used (rollup#339) * Refactored babelHelpers settings - require user declaring which kind of helpers they want to use * Adjust tests to new babelHelpers logic * Keep UNRESOLVED_IMPORT warnings out of the console when running tests * 5.0.0-alpha.0 * Rename skipBabelHelpersCheck to skipPreflightCheck * Make missing babelHelpers a warning * 5.0.0-alpha.1 * Use transformAsync in preflight check * Tweak implementation of the preflight helpers check * Add example to the README how to use the plugin with the Rollup CLI (rollup#327) * explain how to configure the rollup cli * Tweak README * Dont crash on minified code when performing preflight check * Rename rollup-plugin-babel directory to babel * Remove tooling configs not needed in monorepo * Drop transpilation of rollup-plugin-babel, set engines to node@>=10 * Reformat babel plugin with monorepo prettier & ESLint rules * Use exports: named for babel plugin CJS bundle * Migrate babel plugin tests to use ava * Use pnpm to run npm scripts in babel plugin * Use scoped @rollup/plugin-json in babel plugin * Remove unnecessary eslint-disable comment * Update babel plugin metadata to point to monorepo * Remove LICENSE file from babel plugin, pnpm will use the root one when publishing * Move .eslints with turned off rules to fixtures directory * Move loop counter declaration * use getCode helper where possible * Use more proper throwsAsync & notThrowsAsync assertions * Transpile @rollup/plugin-babel using itself * Migrate to use @rollup/pluginutils in babel plugin * Set peer dep range for rollup to ^1.20.0 to align with the rest of plugins * Fix homepage in babel plugin package.json * Include repository reference in test descriptionbs refering to issue numbers * Change URLs in the source files to point to a plugins repo * Change rollup-plugin-babel to @rollup/plugin-babel in source files * cleanup README a little bit * Tweak "Modules" section, remove mentions of Babel 6 from there * Fix some more references to rollup-plugin-babel * Add badges to the top of the README * Adhere to the README template used by other plugins * Tweak CHANGELOG * Mention Babel plugin in the root README * Add publishConfig to package.json of the Babel plugin * Update packages/babel/README.md Co-Authored-By: Tiger Oakes <toakes@mozilla.com> * Update packages/babel/README.md Co-Authored-By: Tiger Oakes <toakes@mozilla.com> * Fixed how @rollup/pluginutils are used * Allow Rollup 2 in peer dependency range * Refactor babel plugin exports shape * tweak usage example in the babel plugin README * Align package.json of Babel plugin to existing package.json files in the monorepo * Update @rollup/pluginutils in the Babel plugin * Add new changelog entries to the Babel plugin * Move babelHelpers recommendation text to the README * Fix typo in the README * chore: remove prepare script, build in prepublishOnly This avoids fresh `pnpm install` problems where the build runs before babel plugins are installed. * Upgrade @babel/preset-env in babel plugin Co-authored-by: Rich-Harris <richard.a.harris@gmail.com> Co-authored-by: Oskar Segersvärd <victorystick@gmail.com> Co-authored-by: Brian Donovan <me@brian-donovan.com> Co-authored-by: Yuta Hiroto <git@about-hiroppy.com> Co-authored-by: Henry Zhu <hi@henryzoo.com> Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> Co-authored-by: Denis <carriere.denis@gmail.com> Co-authored-by: Daniel Tschinder <daniel.tschinder@researchgate.net> Co-authored-by: Brian Ng <bng412@gmail.com> Co-authored-by: Roy Revelt <roy@codsen.com> Co-authored-by: Minh Nguyen <minhnguyenxx@gmail.com> Co-authored-by: Can Göktas <cangoektas@users.noreply.github.com> Co-authored-by: Corey Farrell <git@cfware.com> Co-authored-by: Sebastian Werner <s.werner@sebastian-software.de> Co-authored-by: Bogdan Chadkin <trysound@yandex.ru> Co-authored-by: ylemkimon <mail@ylem.kim> Co-authored-by: Kristoffer <denkristoffer@users.noreply.github.com> Co-authored-by: Shadab Khan <shadab.2005@gmail.com> Co-authored-by: Adam Gołąb <adam@brainhub.pl> Co-authored-by: Kinrany <kinrany@yandex.com> Co-authored-by: Mike Heddes <mikeheddes@gmail.com> Co-authored-by: shellscape <andrew@shellscape.org> Co-authored-by: Ryan Walters <shout@ryanwalters.co> Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com> Co-authored-by: Troy Rhinehart <troy.rhinehart@gmail.com> Co-authored-by: Josh Soref <jsoref@users.noreply.github.com> Co-authored-by: Tiger Oakes <toakes@mozilla.com>
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;
"@rollup/plugin-babel": "^5.2.3" |
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. |
I have made a repro case that shows the same thing: https://replit.com/@stevage/rollup-plugin-repro |
@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 |
Rollup Plugin Name:
@rollup/plugin-babel
This PR contains:
Are tests included?
Breaking Changes?
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.