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

Allow plugins to assert that a specific babel version has loaded the plugin. #7450

Merged
merged 1 commit into from Mar 4, 2018

Conversation

@loganfsmyth
Copy link
Member

commented Feb 28, 2018

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

This allows plugins to use api.assertVersion(7) as a general version, or any semver string to assert that the current version of Babel is one that is supported.

Trying to load @babel/preset-env for instance with Babel 6's CLI results in

Error: Requires Babel "^7.0.0-0", but was loaded with "6.26.0". If you are sure you have a compatible version of @babel/core, it is likely that something in your build process is loading the wrong version. Inspect the stack trace of this error to look for the first entry that doesn't mention "@babel/core" or "babel-core" to see what is calling Babel. (While processing preset: "/Users/<my stuff>/babel/packages/babel-preset-env/lib/index.js")
    at throwVersionError (/Users/<my stuff>/babel/packages/babel-helper-plugin-utils/lib/index.js:45:11)
    at Object.assertVersion (/Users/<my stuff>/babel/packages/babel-helper-plugin-utils/lib/index.js:11:11)
    at _default (/Users/<my stuff>/babel/packages/babel-preset-env/lib/index.js:132:7)
    at /Users/<my stuff>/babel/packages/babel-helper-plugin-utils/lib/index.js:17:12
    at /private/tmp/node_modules/babel-core/lib/transformation/file/options/option-manager.js:317:46
    at Array.map (<anonymous>)
    at OptionManager.resolvePresets (/private/tmp/node_modules/babel-core/lib/transformation/file/options/option-manager.js:275:20)
    at OptionManager.mergePresets (/private/tmp/node_modules/babel-core/lib/transformation/file/options/option-manager.js:264:10)
    at OptionManager.mergeOptions (/private/tmp/node_modules/babel-core/lib/transformation/file/options/option-manager.js:249:14)
    at OptionManager.init (/private/tmp/node_modules/babel-core/lib/transformation/file/options/option-manager.js:368:12)
    at File.initOptions (/private/tmp/node_modules/babel-core/lib/transformation/file/index.js:212:65)
    at new File (/private/tmp/node_modules/babel-core/lib/transformation/file/index.js:135:24)
    at Pipeline.transform (/private/tmp/node_modules/babel-core/lib/transformation/pipeline.js:46:16)
    at transform (/private/tmp/node_modules/babel-cli/lib/babel/util.js:50:22)
    at Object.compile (/private/tmp/node_modules/babel-cli/lib/babel/util.js:59:12)
    at /private/tmp/node_modules/babel-cli/lib/babel/file.js:142:23
    at Array.forEach (<anonymous>)
    at walk (/private/tmp/node_modules/babel-cli/lib/babel/file.js:133:16)
    at files (/private/tmp/node_modules/babel-cli/lib/babel/file.js:156:7)
    at module.exports (/private/tmp/node_modules/babel-cli/lib/babel/file.js:184:5)
    at Object.<anonymous> (/private/tmp/node_modules/babel-cli/lib/babel/index.js:129:1)
    at Module._compile (module.js:660:30)
    at Object.Module._extensions..js (module.js:671:10)
    at Module.load (module.js:573:32)
    at tryModuleLoad (module.js:513:12)

where before it would throw about accessing loose on undefined or something along those lines because the options object isn't passed to plugins in Babel 6.x.

import { types as t } from "@babel/core";

export default function() {
export default declare(api => {
api.assertVersion(7);

This comment has been minimized.

Copy link
@loganfsmyth

loganfsmyth Feb 28, 2018

Author Member

Definitely curious for feedback on this approach. It's a little verbose, but it means that since this is the function that throws, the preset/plugin doing the validation is the thing that throws, so the name of it shows up in the stack trace making debugging a lot nicer.

This comment has been minimized.

Copy link
@probablyup

probablyup Mar 2, 2018

Would it make sense to convert this to "assertSemverVersioning" or something like that? Presumably it will be possible for plugins to be compatible with multiple major versions of Babel.

This comment has been minimized.

Copy link
@loganfsmyth

loganfsmyth Mar 2, 2018

Author Member

Hmm, I worry about that getting pretty verbose. Is the concern that people will assume that assertVersion requires a single version?

api.assertVersion(">=7.x < 9.x") I at least think is acceptable.

This comment has been minimized.

Copy link
@probablyup

probablyup Mar 2, 2018

Is the concern that people will assume that assertVersion requires a single version?

Yeah exactly. When I read assertVersion and saw your example it wasn't immediately obvious that a semver range could be given.

This comment has been minimized.

Copy link
@loganfsmyth

loganfsmyth Mar 2, 2018

Author Member

I feel like documentation would probably be enough to address that, but I do see where you're coming from. I was thinking of the function like assertVersionSatisfies(...) so the version is the Babel core version, not argument string, so the fact that the string can be semver seemed not strongly tied to the mention of version in the name.

At the end of the day, most people bothering to put this assertion in in the first place will probably look at our docs, or else copy paste and not give it a second thought. Since this is only an issue once we start releasing future versions, it seems expected that once it becomes important, people will wonder what specifically it supports as an argument, and at that point the docs are enough.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Feb 28, 2018

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

@danez

This comment has been minimized.

Copy link
Member

commented Mar 4, 2018

I at first thought maybe it would be nicer if modules that contain plugins simple export a second constant which is the version of babel-core that this plugin supports.
But that fails as soon as the plugin is required outside of babel-core which is not an uncommon case I guess, especially with introducing .babelrc.js. So I think it has to be done inside the plugin itself.

I don't think it is too verbose, but if so then we could maybe include the assert in the helper? In the stack the plugin would still how up if I'm not wrong.

export default declare('>=7.2.3', () => plugin);

But I also like how it is now.

@loganfsmyth

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2018

@danez Yeah, I was on the fence about that approach and mostly I opted to go with this PR's approach instead because having it be a programmatic API like .assertVersion means that:

  1. The API is easy to use, even if you're not using @babel/helper-plugin-utils.
  2. Because it is called from inside the plugin, the plugin filename shows up in the stack trace, which helps with debugging.

declare(range, fn) is also an API that could be built on top .assertVersion, but not the other way around, so I figured it would be a better low-level API.

@danez
danez approved these changes Mar 4, 2018

@loganfsmyth loganfsmyth merged commit a479540 into babel:master Mar 4, 2018

4 checks passed

babel/repl REPL preview is available
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 87.27% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@loganfsmyth loganfsmyth deleted the loganfsmyth:babel-version-validation branch Mar 4, 2018

This was referenced Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.