Review @web's preset-env/targets options

[timkelty]

I find it a bit awkward that for @babel/preset-env to use a .browserslistrc file (which is @babel/preset-env's default behavior when no targets is passed), I have to remember to pass targets: false: https://github.com/neutrinojs/neutrino/blob/master/packages/web/index.js#L116-L128

A the very least, I propose we look for a browserslist file, and if one is present, output a notice the console that you may want to pass targets: false if you want to use it.

Thoughts, @neutrinojs/core-contributors ?

[edmorley]

A few thoughts:

1. In the case where the user has a .browserslistrc, would they expect/want Neutrino to use it by default rather than needing to be opt-in?
2. Should Neutrino just use/create a .browserslistrc by default (ie in create-project) itself, and not set targets in @neutrinojs/web? (And make everyone use .browserslistrc)
3. What does create-react-app do?

[timkelty]

I knew it felt like we kind of went over this before…found the existing discussion: #1141

It looks like the browserlist config cannot be JS (config docs), so I guess Neutrino can't offer an output handler for this.

[timkelty]

1. I would think so, but note that it can come from other places too (browserslist key in package.json, BROWSERSLIST env).
2. Interesting idea. Back here you seemed to be in favor of not doing that, as it would depart from .neutrinorc.js being the source of truth.
3. will have to look!

Personally, I don't have a problem with how it works now, but I think a console warning could be nice.

[edmorley]

I'd forgotten we'd discussed in as much depth at that time - doh!

1. note that it can come from other places too

Ah yes I'd forgotten this. TBH I'm not very keen on the package.json/env var locations.

2. [Back here](https://github.com/neutrinojs/neutrino/issues/1141#issuecomment-424666771) you seemed to be in favor of _not_ doing that, as it would depart from `.neutrinorc.js` being the source of truth.

Yeah I guess what made me reconsider is the fact that other tooling (such as eslint compat plugins or ...) uses .browserslistrc but won't know to check the webpack config.

I think a console warning could be nice.

For users that have a .browserslistrc but don't want to use it, would they have a way to suppress the warning? (eg targets: true) Or should we just ignore that scenario - they can always delete the .browserslistrc file if the warning gets annoying?

If we were to go with a console warning this wouldn't be a breaking change, but if we want to change default behaviour it would, so deciding an approach potentially blocks the Neutrino 9 release, so keen to pick soon.

[timkelty]

TBH I'm not very keen on the package.json/env var locations

Agreed.

For users that have a .browserslistrc but don't want to use it, would they have a way to suppress the warning?

I'm thinking just ignore…seems like a rare scenario.

Yeah I guess what made me reconsider is the fact that other tooling (such as eslint compat plugins or ...) uses .browserslistrc but won't know to check the webpack config.

That's why I use it… in addition to preset-env, postcss/autoprefixer uses it.

[timkelty]

What does create-react-app do?

By default, the generated project includes a browserslist configuration in your package.json file: https://create-react-app.dev/docs/supported-browsers-features/#configuring-supported-browsers

So that would be similar to writing a .browserslistrc file, which sounds like we both prefer vs. env/package.json

[timkelty]

I think my vote is for……………writing a .browerslistrc file? 😬

[edmorley]

At this point I'm open to pretty much any of the options! Could you describe a bit more about migration path for existing vs new projects?

[timkelty]

@edmorley for .neutrinorc.js migration…off the top of my head:

• if web.targets !== false, do nothing. The user is directly passing targets, which is fine.
• else:
  • if web.targets === false and there is already a .browserslistrc file, remove the targets: false option
  • else if web.targets is not set, generate a .browserslistrc file with the defaults

[edmorley]

Sounds good to me; want to open a PR? :-)

[timkelty]

yep – should be able to get something today/tomorrow.

[timkelty]

Sorry got slammed this week…I'll try to fit this in yet, but no promises :(

[timkelty]

@edmorley still trying to get to this this week, but let me know where you think it stands relative to the v9 release.

Obviously it's breaking and would be good to get it in, but wouldn't be a horrible change to wait for v10 for if it is the only thing preventing a v9 release.

That said, I've been on v9 RCs for a long time so it doesn't really affect me, but I know others are itching for an official release.

[edmorley]

Thinking about this more, it's likely going to take a few iterations of polishing migration/UX/docs, which would mean needing several more RCs. As such, I think it's best if this doesn't block Neutrino 9.0.0 final. We can always publish a v9 point release in the future that uses one of the non-breaking options discussed above to improve the issue here until Neutrino 10 is released :-)

[timkelty]

@edmorley I think that's a good call. I'll work on a PR for v10 soon enough.