Skip to content
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

Upgrade from rollup-plugin-babel to @rollup/plugin-babel #789

Merged

Conversation

vladdy-moses
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Aug 9, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/formium/tsdx/ff7ar8nbs
✅ Preview: https://tsdx-git-fork-vladdy-moses-update-rollup-babel-plugin.formium.vercel.app

@vladdy-moses
Copy link
Contributor Author

Also fixed Node CI / Lint on node 10.x and ubuntu-latest (pull_request) check.

@vladdy-moses vladdy-moses marked this pull request as ready for review August 9, 2020 15:19
@agilgur5 agilgur5 added version: minor Increment the minor version when merged topic: Node 10+ requires Node 10+ labels Aug 9, 2020
@jaredpalmer

This comment has been minimized.

@agilgur5

This comment has been minimized.

@agilgur5 agilgur5 changed the title Migrate from rollup-plugin-babel to @rollup/plugin-babel Upgrade from rollup-plugin-babel to @rollup/plugin-babel Aug 11, 2020
Copy link
Collaborator

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladdy-moses thanks for the PR!

PR Suggestions

Please ensure to write a commit message and PR description next time; it would make reviewing a lot easier if you linked to the changelog and relevant breaking changes in your message. I've added the appropriate references in comments below.
A rationale would also be good to have. I assume this is just to get rid of a deprecation notice, however it would be better not to assume.

Changes Required

Please remove the CI change per the in-line comment. The rest of the comments are for reference.

Mergeability

This requires Node 10+, which also wasn't mentioned as a breaking change in the commit message. I've marked it as such and milestone'd it for v0.14.0 to go along with the other Node 10+ dep changes and official drop of Node <10 there.

src/babelPluginTsdx.ts Show resolved Hide resolved
src/createRollupConfig.ts Show resolved Hide resolved
.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
yarn.lock Show resolved Hide resolved
@agilgur5
Copy link
Collaborator

Have taken over this myself since this has been stale for several weeks and is one of the last dep updates being added to v0.14.0, which is just about ready otherwise. This also fits into the theme as an upgrade that requires Node 10+.

I've removed the change I requested to be removed, rebased the PR and fixed the yarn.lock and package.json conflicts, and tests are looking to pass now.

I assume this is just to get rid of a deprecation notice, however it would be better not to assume.

Yup, seems to be so, got this warning on a fresh install of v0.13.3:

npm WARN deprecated rollup-plugin-babel@4.4.0: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-babel.

BREAKING: requires Node 10+

- was getting a warning on install:
  - "npm WARN deprecated rollup-plugin-babel@4.4.0: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-babel."

- c.f. https://github.com/rollup/plugins/blob/master/packages/babel/CHANGELOG.md#breaking-changes
  - `babelPlugin.custom` is now a separate export,
    `createBabelInputPluginFactory`
  - `babelHelpers: 'bundled'` was added
    - this is the default and backward-compatible with 4.x, but they
      recommended explicitly configuring it as such

- c.f. https://github.com/rollup/plugins/blob/master/packages/babel/CHANGELOG.md#v510
  - typings have been added to the package directly, no need to declare
    module to workaround lack of typings anymore

- remove old commented out line referring to rollup-plugin-babel as well

Co-Authored-By: Anton Gilgur <agilgur5@gmail.com>
@agilgur5
Copy link
Collaborator

Removed the two leftover references to rollup-plugin-babel, one was an old commented out import, another was a declare module, which is no longer necessary because @rollup/plugin-babel ships its own typings.

@agilgur5 agilgur5 merged commit f592595 into jaredpalmer:master Sep 21, 2020
@agilgur5
Copy link
Collaborator

@allcontributors please add @vladdy-moses for code

@allcontributors
Copy link
Contributor

@agilgur5

I've put up a pull request to add @vladdy-moses! 🎉

paul-vd pushed a commit to EezyQuote/tsdx that referenced this pull request Dec 1, 2020
…#789)

BREAKING: requires Node 10+

- was getting a warning on install:
  - "npm WARN deprecated rollup-plugin-babel@4.4.0: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-babel."

- c.f. https://github.com/rollup/plugins/blob/master/packages/babel/CHANGELOG.md#breaking-changes
  - `babelPlugin.custom` is now a separate export,
    `createBabelInputPluginFactory`
  - `babelHelpers: 'bundled'` was added
    - this is the default and backward-compatible with 4.x, but they
      recommended explicitly configuring it as such

- c.f. https://github.com/rollup/plugins/blob/master/packages/babel/CHANGELOG.md#v510
  - typings have been added to the package directly, no need to declare
    module to workaround lack of typings anymore

- remove old commented out line referring to rollup-plugin-babel as well

Co-Authored-By: Anton Gilgur <agilgur5@gmail.com>

Co-authored-by: Anton Gilgur <agilgur5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problem: stale Issue has not been responded to in some time scope: dependencies Pull requests that update a dependency file topic: Node 10+ requires Node 10+ version: minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants