Add resolvePlugin and resolvePreset methods to babel-core API #4729

rmacklin · 2016-10-15T00:22:02Z

Q	A
Bug fix?	no
Breaking change?	no
New feature?	yes
Deprecations?	no
Spec compliancy?	no
Tests added/pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	will create one once merged

This PR adds resolvePlugin and resolvePreset methods to the babel-core API.

Exposing these publicly is useful because it allows plugins/preset names to be mapped to their absolute paths, which is necessary if babel.transform will process a file that is not a descendant of the directory in which the babel plugins/presets were installed^[1]. (Otherwise, babel will fail to resolve the plugins/presets because it'll try resolve from the processed file's dirname.)

^[1] One use case for this (the one that prompted me to make this change) is to work with ruby gems that include javascript assets (which is common among Rails plugins). Unlike node modules, ruby gems are installed globally, which means those javascript files won't be inside the application subtree (where your babel plugins and presets are installed). I'm developing a sprockets plugin to allow transpiling via babel 6 (sprockets-es6 is stuck on babel 5) and with these methods I can transform the user-supplied plugins/presets to their absolute paths so that processing assets inside globally-installed ruby gems will work.

rmacklin · 2016-10-15T00:22:28Z

Supersedes #4612

codecov-io · 2016-10-15T01:59:46Z

Current coverage is 89.94% (diff: 100%)

No coverage report found for master at 2dc919d.

Powered by Codecov. Last update 2dc919d...6ec93d7

rmacklin · 2016-10-15T02:55:57Z

Well, the tests I added are failing on old node versions (since resolve doesn't go up the directory tree to find transform-es2015-arrow-functions in the root node_modules). I could test this differently by just checking what gets passed to resolve, but to do that I'd need to add something like sinon spies, which we probably don't want. Thoughts?

rmacklin · 2016-10-15T22:02:22Z

Another option is to add another layer of abstraction, which encapsulates only the logic determining what strings we want to pass to resolve. Originally I didn't want to do this because of the extra level of indirection, but ultimately the extracted logic is precisely what we want to test, and doing so doesn't involve calling resolve in the tests, so it works on any node version.

I just pushed some new commits with this refactoring to extract and test these two methods: getPossiblePluginNames and getPossiblePresetNames and CI is now passing. And now that I've done it, I don't have issues with the extra layer of indirection because resolvePlugin and resolvePreset became "dead simple" wrappers.

rmacklin · 2016-10-15T23:31:30Z

Rebased to resolve merge conflicts with #4734. Since I was already rebasing, I squashed out 6592c2c and 6df614d to make the history cleaner and allow each commit to pass CI.

This encapsulates the logic for turning an acceptable plugin name into the absolute path for that plugin. It can be used to preprocess a plugins list to map each plugin to its absolute path, which is necessary if `babel.transform` is going to be executed on a file outside the directory subtree where the plugins are installed. This adds a getPossiblePluginNames helper encapsulating the logic for what plugin names we should try to resolve, and the resolvePlugin method just calls this helper and actually resolves them.

This encapsulates the logic for turning an acceptable preset name into the absolute path for that preset. It can be used to preprocess a presets list to map each preset to its absolute path, which is necessary if `babel.transform` is going to be executed on a file outside the directory subtree where the presets are installed. This adds a getPossiblePresetNames helper encapsulating the logic for what preset names we should try to resolve, and the resolvePreset method just calls this helper and actually resolves them.

Previously these were testing the logic that is now encapsulated in getPossiblePresetNames and tested in a unit test

"DRY"s the duplicated algorithm in resolvePlugin and resolvePreset

rmacklin · 2016-11-14T04:54:36Z

@loganfsmyth @hzoo @kaicataldo @DrewML (or any other maintainers that can look at this) Can we merge this?

(And sorry if you got duplicate notifications...had some issues posting from my phone)

hzoo · 2016-12-08T17:39:17Z

packages/babel-core/test/get-possible-preset-names.js

+  });
+
+  it("inserts babel-preset after @org/", function() {
+    assert.deepEqual(getPossiblePresetNames("@babel/es2015"), [


I'm kind of thinking if you did @babel/es2015 you probably would ever mean "babel-preset-@babel/es2015", so we should remove this option?

I agree, but in this PR I just wanted to keep things as straightforward as possible, so I preserved the existing behavior. I can remove that in a separate PR if there's consensus on it after this one is merged.

loganfsmyth · 2016-12-20T16:42:16Z

This looks good to me.

hzoo · 2017-01-20T01:18:49Z

Released in https://github.com/babel/babel/releases/tag/v6.22.0

loganfsmyth · 2017-03-26T01:31:41Z

@rmacklin Beyond making the transition from this code to what you wanted most easily, was there a specific reason you made these helpers return null? I'm thinking it might be nicer to have them throw if the name can't be resolved, in Babel 7.x.

rmacklin · 2017-03-28T04:30:34Z

@loganfsmyth Looking at this code again, I think resolvePlugin and resolvePreset returned null because they replaced the previous uses of resolve, which was returning null if an error was caught.

It's probably fine to change them to throw in Babel 7.x - if someone wanted null instead they could write a small wrapper that catches the error.

rmacklin mentioned this pull request Oct 15, 2016

Add pluginResolutionDir option #4612

Closed

rmacklin changed the title ~~Add resolve plugin and resolve preset~~ Add resolvePlugin and resolvePreset Oct 15, 2016

rmacklin force-pushed the add-resolvePlugin-and-resolvePreset branch from 0df3823 to 3aa3814 Compare October 15, 2016 02:04

rmacklin force-pushed the add-resolvePlugin-and-resolvePreset branch from ca520ba to 6016cac Compare October 15, 2016 22:06

rmacklin changed the title ~~Add resolvePlugin and resolvePreset~~ Add resolvePlugin and resolvePreset methods to babel-core API Oct 15, 2016

rmacklin force-pushed the add-resolvePlugin-and-resolvePreset branch from 6016cac to ba0c3d1 Compare October 15, 2016 23:30

rmacklin force-pushed the add-resolvePlugin-and-resolvePreset branch 2 times, most recently from 0908a2e to 9f28d4d Compare October 15, 2016 23:44

rmacklin added 3 commits October 16, 2016 10:52

Remove unneeded tests

4ea1007

Previously these were testing the logic that is now encapsulated in getPossiblePresetNames and tested in a unit test

rmacklin force-pushed the add-resolvePlugin-and-resolvePreset branch from 9f28d4d to f095ef1 Compare October 16, 2016 18:11

Extract resolveFromPossibleNames to remove duplicated logic

6ec93d7

"DRY"s the duplicated algorithm in resolvePlugin and resolvePreset

rmacklin force-pushed the add-resolvePlugin-and-resolvePreset branch from f095ef1 to 6ec93d7 Compare October 16, 2016 18:19

hzoo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Oct 21, 2016

rmacklin closed this Nov 14, 2016

rmacklin reopened this Nov 14, 2016

hzoo reviewed Dec 8, 2016

View reviewed changes

loganfsmyth merged commit ce0c620 into babel:master Dec 20, 2016

rmacklin deleted the add-resolvePlugin-and-resolvePreset branch December 20, 2016 18:03

rhoggSugarcrm mentioned this pull request Jan 26, 2017

Upgrade babel sugarcrm/thorn#102

Merged

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resolvePlugin and resolvePreset methods to babel-core API #4729

Add resolvePlugin and resolvePreset methods to babel-core API #4729

rmacklin commented Oct 15, 2016

rmacklin commented Oct 15, 2016

codecov-io commented Oct 15, 2016 •

edited

rmacklin commented Oct 15, 2016 •

edited

rmacklin commented Oct 15, 2016 •

edited

rmacklin commented Oct 15, 2016 •

edited

rmacklin commented Nov 14, 2016 •

edited

hzoo Dec 8, 2016

rmacklin Dec 8, 2016 •

edited

loganfsmyth commented Dec 20, 2016

hzoo commented Jan 20, 2017

loganfsmyth commented Mar 26, 2017

rmacklin commented Mar 28, 2017 •

edited

Add resolvePlugin and resolvePreset methods to babel-core API #4729

Add resolvePlugin and resolvePreset methods to babel-core API #4729

Conversation

rmacklin commented Oct 15, 2016

rmacklin commented Oct 15, 2016

codecov-io commented Oct 15, 2016 • edited

Current coverage is 89.94% (diff: 100%)

rmacklin commented Oct 15, 2016 • edited

rmacklin commented Oct 15, 2016 • edited

rmacklin commented Oct 15, 2016 • edited

rmacklin commented Nov 14, 2016 • edited

hzoo Dec 8, 2016

Choose a reason for hiding this comment

rmacklin Dec 8, 2016 • edited

Choose a reason for hiding this comment

loganfsmyth commented Dec 20, 2016

hzoo commented Jan 20, 2017

loganfsmyth commented Mar 26, 2017

rmacklin commented Mar 28, 2017 • edited

codecov-io commented Oct 15, 2016 •

edited

rmacklin commented Oct 15, 2016 •

edited

rmacklin commented Oct 15, 2016 •

edited

rmacklin commented Oct 15, 2016 •

edited

rmacklin commented Nov 14, 2016 •

edited

rmacklin Dec 8, 2016 •

edited

rmacklin commented Mar 28, 2017 •

edited