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

Remove Stage presets #8293

Merged
merged 8 commits into from Jul 24, 2018
Merged

Remove Stage presets #8293

merged 8 commits into from Jul 24, 2018

Conversation

hzoo
Copy link
Member

@hzoo hzoo commented Jul 9, 2018

Sad times 😭, but for the best? Fun because I added them back in originally 😛

Fixes #7770

We should prob make a better guide on making your own preset

@hzoo hzoo added this to the Babel 7 RC milestone Jul 9, 2018
@babel-bot
Copy link
Collaborator

babel-bot commented Jul 9, 2018

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

@hzoo hzoo requested a review from nicolo-ribaudo July 9, 2018 21:26
@hzoo hzoo added the PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release label Jul 10, 2018
@hzoo hzoo requested review from xtuc and Andarist July 10, 2018 19:27
Copy link
Member

@chicoxyzzy chicoxyzzy left a comment

Choose a reason for hiding this comment

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

Overall this LGTM. Not sure about readmes, where I think we can add list of plugins down to current preset instead of higher stage preset + list of plugins for current preset.

@hzoo
Copy link
Member Author

hzoo commented Jul 11, 2018

I think we can add list of plugins down to current preset instead of higher stage preset + list of plugins for current preset.

Yeah I was thinking that too.


---

If you want the same configuration as before, you can use this configuration, although keep in mind that Stage 0 contains Stage 1 which is also deprecated.
Copy link
Member

Choose a reason for hiding this comment

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

This lines in readmes should be reworded somehow (also probably same for deprecation messages) =)


if (opts !== undefined) {
if (opts.loose !== undefined) loose = opts.loose;
}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We can just use { loose = false } = {} instead of the opts parameter to avoid lines 7-11.

const { loose = false, useBuiltIns = false, decoratorsLegacy = false } = opts;

return {
presets: [[presetStage3, { loose, useBuiltIns }]],
Copy link
Member

Choose a reason for hiding this comment

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

useBuiltIns can be removed, since it was only used by object-rest-spread.
It can also be removed in the stage-0 and stage-1 presets.

proposals.join(", "),
);
}
We recommend that make your own presets to use across projects for
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

We recommend that you make your own presets...

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Also, you can remove { "loose": false } everywhere since it is the default.

```

or using yarn:
We recommend that make your own presets to use across projects for
Copy link
Member

Choose a reason for hiding this comment

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

Is this something we want to recommend? I'm not sure how I feel about this being in here as a recommendation for good.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm yeah, I guess it shouldn't have to be a recommendation but more that people tend to forget you can make your own

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm fine if we want to say that creating a custom preset is an option, I think the wording right now just makes it sound like what we're expecting everyone to do, which isn't good.

Maybe something like

If you're using the same configuration across many separate projects, keep in mind that you can also create your own custom presets with whichever plugins and presets you're looking to use.

@@ -35,19 +34,6 @@ export default (_, opts) => {
if (opts.spec !== undefined) spec = opts.spec;
}

if (typeof loose !== "boolean") {
Copy link
Member

Choose a reason for hiding this comment

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

What happened with this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh hmm I think I ended up removing this since it's for standalone and it would be a UI option in the repl, but we can add it back too.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I was just confused because this is only the validation logic, not the options themselves.

module.exports = function() {
return {
plugins: [
// ...
Copy link
Member

Choose a reason for hiding this comment

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

If we want this example, we should probably also clarify that doing require() on these is recommended. If they just do "@babel/plugin-proposal-function-bind" as a string, the plugins will probably not be found properly. I'd actually thought about making core error when that happens, but I haven't yet. Maybe I should?

calebeby pushed a commit to Pigmice2733/scouting-frontend that referenced this pull request Jul 17, 2018
This Pull Request renovates the package group "babel monorepo".


-   [@​babel/preset-typescript](https://github.com/babel/babel) (`devDependencies`): from `7.0.0-beta.53` to `7.0.0-beta.54`
-   [@​babel/preset-env](https://github.com/babel/babel) (`devDependencies`): from `7.0.0-beta.53` to `7.0.0-beta.54`
-   [@​babel/plugin-transform-react-jsx](https://github.com/babel/babel) (`devDependencies`): from `7.0.0-beta.53` to `7.0.0-beta.54`
-   [@​babel/plugin-proposal-class-properties](https://github.com/babel/babel) (`devDependencies`): from `7.0.0-beta.53` to `7.0.0-beta.54`
-   [@​babel/core](https://github.com/babel/babel) (`devDependencies`): from `7.0.0-beta.53` to `7.0.0-beta.54`

# Release Notes
<details>
<summary>babel/babel</summary>

### [`v7.0.0-beta.54`](https://github.com/babel/babel/releases/v7.0.0-beta.54)
[Compare Source](babel/babel@v7.0.0-beta.53...v7.0.0-beta.54)
#### v7.0.0-beta.54 (2018-07-16)

> Regarding `babel/babel#8184, we aren't using `micromatch` for paths, just basic `*/**` substitution now. For anything more complicated we will recommend using a regex/.js config.
> There was a bug in the stage presets (`babel/babel#8307), so we just removed the requirements for setting options in the meantime for ease of use. We are removing the Stage presets next anyway. `babel/babel#8293
##### 💥 Breaking Change
* `babel-core`, `babel-register`, `babel-traverse`
  * [#&#8203;8327](`babel/babel#8327) Treat string ignore/only/test/include/exclude values as paths with only basic pattern matching. ([@&#8203;loganfsmyth])
##### 🐛 Bug Fix
* `babel-core`, `babel-register`, `babel-traverse`
  * [#&#8203;8327](`babel/babel#8327) Treat string ignore/only/test/include/exclude values as paths with only basic pattern matching. ([@&#8203;loganfsmyth])
* `babel-preset-stage-0`, `babel-preset-stage-1`
  * [#&#8203;8317](`babel/babel#8317) Fix stage-0/1 import of pipeline proposals array. ([@&#8203;mAAdhaTTah])
* `babel-helper-module-transforms`, `babel-plugin-transform-modules-commonjs`
  * [#&#8203;8316](`babel/babel#8316) Ensure that the wildcard interop is used with re-export + default.. ([@&#8203;loganfsmyth])
* `babel-core`
  * [#&#8203;8315](`babel/babel#8315) Remove option-filtering options from the final options results.. ([@&#8203;loganfsmyth])
##### 📝 Documentation
* [#&#8203;8320](`babel/babel#8320) Add link to audio version of song. ([@&#8203;rugk])
##### Committers: 4
- Daniel Tschinder ([danez])
- James DiGioia ([mAAdhaTTah])
- Logan Smyth ([loganfsmyth])
- rugk ([rugk])

---


</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
```json
{
"plugins": [
// Stage 0
Copy link
Member

@xtuc xtuc Jul 23, 2018

Choose a reason for hiding this comment

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

Comments in JSON are not valid, could we remove them? (because user will need to) or can we switch to js?

Following READMEs too

Copy link
Member

Choose a reason for hiding this comment

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

.babelrc uses Json5, so comments work.

@@ -66,6 +66,7 @@ yarn add ${name} --dev

packages
.filter(x => x !== "README.md") // ignore root readme
.filter(x => x.indexOf("babel-preset-stage-") === -1) // ignore stages
Copy link
Member

Choose a reason for hiding this comment

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

This seems obscure to me. I would prefer using a rejection list which explicitly exclude: ['babel-preset-stage-0', 'babel-preset-stage-1', 'babel-preset-stage-2']

Copy link
Member Author

Choose a reason for hiding this comment

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

That's fine but just FYI this is a temporary change because we are going to not publish any more of the package anyway so I would remove this code the next release after.

@xtuc
Copy link
Member

xtuc commented Jul 23, 2018

Could we also remove the stage plugins from the REPL? I think that the possibility of loading plugins from npm would suffice? IRC parser flags were not pushed, we might need to fix that first.

@hzoo
Copy link
Member Author

hzoo commented Jul 23, 2018

Removing from repl isn't part of this PR since I left it in babel-standalone so we are leaving it for now? Yeah would like to figure out how we want to go about doing that. Don't have a good solution and is simple to do atm. I think the use case for the repl is different since it's fine to just allow any syntax mostly and we can easily keep it up to date since it's just for testing

@mAAdhaTTah
Copy link
Contributor

It might be confusing for people to find presets on the repl that don't exist / shouldn't be used anymore.

@hzoo
Copy link
Member Author

hzoo commented Jul 23, 2018

I think that's fine, this is only for v7 while we are figuring out what to do there. We have the same problem with loose mode, plugin options.

@hzoo hzoo merged commit c70a32a into master Jul 24, 2018
@xtuc xtuc deleted the rm-stage-presets branch July 24, 2018 05:28
hzoo pushed a commit to babel/babel-upgrade that referenced this pull request Jul 25, 2018
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should we remove Stage 0 - 2 presets, leaving only stage 3?
9 participants