[7.0] Remove bc code from preset handling and preset-es2015 #5128

danez · 2017-01-15T17:15:17Z

Q	A
Patch: Bug Fix?	n
Major: Breaking Change?	y
Minor: New Feature?	n
Deprecations?	y
Spec Compliancy?	n
Tests Added/Pass?	y
Fixed Tickets
License	MIT
Dependency Changes

For bc we supported exporting an object + function (as buildPreset) at the same time. This now removes the bc and a preset needs to export either an object or a function as default export or module.export.

mention-bot · 2017-01-15T17:15:18Z

@danez, thanks for your PR! By analyzing the history of the files in this pull request, we identified @loganfsmyth, @motiz88 and @hzoo to be potential reviewers.

codecov-io · 2017-01-15T17:31:40Z

Codecov Report

Merging #5128 into 7.0 will increase coverage by 0.04%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##              7.0    #5128      +/-   ##
==========================================
+ Coverage   90.38%   90.43%   +0.04%     
==========================================
  Files         200      208       +8     
  Lines        9925     9938      +13     
  Branches     2697     2695       -2     
==========================================
+ Hits         8971     8987      +16     
+ Misses        954      951       -3

Impacted Files	Coverage Δ
packages/babel-preset-es2015/src/index.js	`94.73% <100%> (-0.51%)`	❌
packages/babel-preset-flow/src/index.js	`100% <100%> (ø)`
packages/babel-preset-es2017/src/index.js	`100% <100%> (ø)`
packages/babel-preset-react/src/index.js	`100% <100%> (ø)`
packages/babel-preset-stage-0/src/index.js	`100% <100%> (ø)`
packages/babel-preset-stage-1/src/index.js	`100% <100%> (ø)`
packages/babel-preset-latest/src/index.js	`100% <100%> (ø)`	✅
packages/babel-preset-stage-2/src/index.js	`100% <100%> (ø)`
packages/babel-preset-stage-3/src/index.js	`100% <100%> (ø)`
packages/babel-preset-es2016/src/index.js	`100% <100%> (ø)`
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9f01cb...274db46. Read the comment docs.

loganfsmyth · 2017-01-15T20:47:59Z

packages/babel-core/src/transformation/file/options/option-manager.js

-        if (typeof val !== "function" && options !== undefined) {
-          throw new Error(`Options ${JSON.stringify(options)} passed to ` +
-            (presetLoc || "a preset") + " which does not accept options.");
+        if (typeof val === "function") {


Should we standardize and require presets to be functions, and drop Object support too?

That would make it even simpler. And 3rd-party presets could still be bc down to 6.13 as functions are supported since then anyway.

What do you think? @hzoo, @DrewML, @xtuc

If we can simply the logic (this code is the worst and so many hacks) then ok 👍 just want to make sure its easy to change it. We can write a migration guide + even try a codemod if that's useful.

👍 for keeping stuff as simple as possible.

danez · 2017-01-16T23:07:57Z

Okay, I made all the changes.

With this changes we don't have a way currently to detect if the preset supports options or not. I tried Function.length but that wasn't working for functions with default params. Maybe after 7.0 we can add some option-schema like eslint has. That would solve this.

This case will now not throw a specific message like before but rather : Unsupported preset format: object. Expected preset to return a function.

{
   "preset": [{ option: true }],
}

xtuc · 2017-01-17T07:02:34Z

Just an idea but I think that codeframes helps a lot new users.

The error would be like:

Error: Unsupported preset format: object. Expected preset to return a function
{
   "preset": [{ option: true }],
                ^^^^^^^^^^^^
}

Could also be applied to parse errors.

What do you think?

danez · 2017-01-17T09:47:17Z

Yes, but could also be done post-7.

hzoo · 2017-01-21T22:25:14Z

any reviews for this?

hzoo · 2017-01-23T23:28:29Z

Only allow functions for presets

Is it possible to write a codemod for this for any easy presets? Otherwise just needs to be in the migration guide for users

xtuc · 2017-01-24T08:00:14Z

👍 The migration guide. I have this as todo in the guide. I don't exactly know how to write it.

hzoo · 2017-01-26T23:00:15Z

Maybe after 7.0 we can add some option-schema like eslint has.

Yeah we should probably do this otherwise we get typos issues.

danez · 2017-02-20T15:09:20Z

What should we do here? Do we want to merge this? I can rebase if so

hzoo · 2017-02-20T15:09:50Z

I don't anyone was opposed so yea!

danez added the PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release label Jan 15, 2017

danez added this to the Babel 7 milestone Jan 15, 2017

danez changed the base branch from master to 7.0 January 15, 2017 19:39

loganfsmyth reviewed Jan 15, 2017

View reviewed changes

danez and others added 4 commits February 20, 2017 16:13

Remove bc code from preset handling and preset-es2015

8010f2d

Add more tests

0547505

Only allow functions for presets

762eca5

Fix lint

274db46

xtuc approved these changes Feb 22, 2017

View reviewed changes

danez merged commit 87ca615 into babel:7.0 Feb 22, 2017

danez deleted the remove-bc-from-presets branch February 22, 2017 13:58

This was referenced Feb 23, 2017

Merge master into 7.0 #5368

Merged

Revert "[7.0] Remove bc code from preset handling and preset-es2015" #5375

Closed

xtuc mentioned this pull request Feb 27, 2017

Make babel-node a standalone package #5279

Closed

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

[7.0] Remove bc code from preset handling and preset-es2015 #5128

[7.0] Remove bc code from preset handling and preset-es2015 #5128

danez commented Jan 15, 2017

mention-bot commented Jan 15, 2017

codecov-io commented Jan 15, 2017 •

edited by codecov bot

loganfsmyth Jan 15, 2017

danez Jan 16, 2017 •

edited

hzoo Jan 16, 2017

existentialism Jan 16, 2017

xtuc Jan 17, 2017

danez commented Jan 16, 2017

xtuc commented Jan 17, 2017 •

edited

danez commented Jan 17, 2017

hzoo commented Jan 21, 2017

hzoo commented Jan 23, 2017

xtuc commented Jan 24, 2017 •

edited

hzoo commented Jan 26, 2017

danez commented Feb 20, 2017

hzoo commented Feb 20, 2017 •

edited

[7.0] Remove bc code from preset handling and preset-es2015 #5128

[7.0] Remove bc code from preset handling and preset-es2015 #5128

Conversation

danez commented Jan 15, 2017

mention-bot commented Jan 15, 2017

codecov-io commented Jan 15, 2017 • edited by codecov bot

Codecov Report

loganfsmyth Jan 15, 2017

Choose a reason for hiding this comment

danez Jan 16, 2017 • edited

Choose a reason for hiding this comment

hzoo Jan 16, 2017

Choose a reason for hiding this comment

existentialism Jan 16, 2017

Choose a reason for hiding this comment

xtuc Jan 17, 2017

Choose a reason for hiding this comment

danez commented Jan 16, 2017

xtuc commented Jan 17, 2017 • edited

danez commented Jan 17, 2017

hzoo commented Jan 21, 2017

hzoo commented Jan 23, 2017

xtuc commented Jan 24, 2017 • edited

hzoo commented Jan 26, 2017

danez commented Feb 20, 2017

hzoo commented Feb 20, 2017 • edited

codecov-io commented Jan 15, 2017 •

edited by codecov bot

danez Jan 16, 2017 •

edited

xtuc commented Jan 17, 2017 •

edited

xtuc commented Jan 24, 2017 •

edited

hzoo commented Feb 20, 2017 •

edited