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

Restrict the use of passPerPreset #6921

Open
taion opened this Issue Nov 27, 2017 · 5 comments

Comments

Projects
None yet
4 participants
@taion
Contributor

taion commented Nov 27, 2017

cc @loganfsmyth

Feature change request

Input Code

N/A

Babel/Babylon Configuration (.babelrc, package.json, cli command)

Anything with passPerPreset

Expected Behavior

The use of passPerPreset should be restricted

Current Behavior

passPerPreset is allowed in user configurations

Possible Solution

Disallow using passPerPreset in user Babel configurations.

Context

The majority of current usages of passPerPreset in user code originally date to a misunderstanding about plugin ordering with Relay. Relay examples now no longer use passPerPreset: relayjs/relay-starter-kit#102, and I've opened an issue with the Apollo devs to remove derived usages there as well: apollographql/starwars-server#5.

Correspondingly, there may not be any real use cases for passPerPreset in user code any more. As such, given that this feature has historically not been used very often, and has led to odd interactions between existing presets due to just using a rare code path (e.g. #4162), I think it would make sense to heavily limit its use.

Even in cases where it does arise, it's not the correct scope – the scope instead looks more like "this preset or this plugin should have its own pass", rather than "every preset or plugin in this config (or every plugin in this preset) should have its own pass".

Your Environment

N/A

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Nov 27, 2017

Collaborator

Hey @taion! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

Collaborator

babel-bot commented Nov 27, 2017

Hey @taion! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@xtuc

This comment has been minimized.

Show comment
Hide comment
@xtuc

xtuc Dec 1, 2017

Member

I agree that we should restrict its usage. After a quick search it seems that some users are relying on it GitHub search even if it's not documented.

We can introduce a warning in Babel 7 and drop it in Babel 8, what do you think?

Member

xtuc commented Dec 1, 2017

I agree that we should restrict its usage. After a quick search it seems that some users are relying on it GitHub search even if it's not documented.

We can introduce a warning in Babel 7 and drop it in Babel 8, what do you think?

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Dec 1, 2017

Contributor

You can also see that substantially all those cases are GraphQL-related cases, owing ultimately to that initial Relay thing.

I think @loganfsmyth mentioned that babel-minify still needed it? Ultimately I think passPerPreset is sort of the wrong concept – like I said above, I can see there being a valuable idea in "this preset runs in its own pass", but ultimately I don't think this option should be exposed to end-users like this.

Contributor

taion commented Dec 1, 2017

You can also see that substantially all those cases are GraphQL-related cases, owing ultimately to that initial Relay thing.

I think @loganfsmyth mentioned that babel-minify still needed it? Ultimately I think passPerPreset is sort of the wrong concept – like I said above, I can see there being a valuable idea in "this preset runs in its own pass", but ultimately I don't think this option should be exposed to end-users like this.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Dec 1, 2017

Member

We said we should at least only allow it in a preset (like minify) then end-users wouldn't be able to set it themselves.

Member

hzoo commented Dec 1, 2017

We said we should at least only allow it in a preset (like minify) then end-users wouldn't be able to set it themselves.

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Dec 1, 2017

Contributor

Even for minify, one goal would be to run the entire minify preset after all the earlier presets, no? I feel like there are a couple of different concepts here that are a little bit mixed up:

  • "Run this preset in its own pass"
  • "Run each part of this preset in its own pass"
Contributor

taion commented Dec 1, 2017

Even for minify, one goal would be to run the entire minify preset after all the earlier presets, no? I feel like there are a couple of different concepts here that are a little bit mixed up:

  • "Run this preset in its own pass"
  • "Run each part of this preset in its own pass"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment