Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Allow browserslistsrc file to configure babel targets #1141

Closed
timkelty opened this issue Sep 26, 2018 · 11 comments
Closed

Allow browserslistsrc file to configure babel targets #1141

timkelty opened this issue Sep 26, 2018 · 11 comments
Assignees
Labels
Milestone

Comments

@timkelty
Copy link
Contributor

A while back I brought this up: #229

It is desirable to have a browserslist file so various tools can pick use it (autoprefixer, stylelint, eslint-plugin-compat, and of course, @babel/preset-env).

@babel/preset-env supports this out of the box: https://babeljs.io/docs/en/babel-preset-env#browserslist-integration, however, since Neutrino passes targets directly, I'm assuming it will take those values and not read them from the file.

I'm not sure if this is a situation where we can use neutrinorc as the single source of truth, as .browserslistrc files, I believe are just plain text (no js).

@timkelty timkelty added this to the v9 milestone Sep 26, 2018
@timkelty
Copy link
Contributor Author

A potential downside is confusion over what you're targeting (browsers vs node), when building a server and a frontend.

My current workaround is to maintain a .browserslistrc file, and manually call the browerslist function like so:

const browsers = require('browserslist')();

module.exports = {
  options: {
    root: __dirname,
  },
  use: [
    [
      '@neutrinojs/web',
      {
        targets: {
          browsers
        }
      }
    ]
  ]
};

Not a bad solution, but I think we should take an official stance on this, and perhaps document an example if people do want to use a browerslistrc file.

@edmorley
Copy link
Member

Hi!

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

As such, the options seem to be:

  1. Document how to use the browserlist config instead of Neutrino's targets
  2. Adjust the Neutrino presets to look for a browserlist config, and if found, don't pass targets to @babel/preset-env, to allow its default behaviour to work
  3. Switch entirely to the browserlist config and don't configure it via .neutrinorc.js at all

For (1), we could make this simpler to do, by supporting passing targets: false, that way Babel's default behaviour kicks in, and no need for the manual require('browserslist')().

For (2), there are lots of different ways the config can be set, so we'd presumably have to just use the browserlist package to do this - though it's already bundled with @babel/preset-env (and many others), so it at least wouldn't increase the overall size of the dependency tree. As far as UX goes, I imagine there are people who are surprised Neutrino doesn't already read their browserlist config, but also others who might not realise they even have one (eg leftover in their package.json after trying another tool) and then get confused when Neutrino doesn't support the browsers we say we do in the preset READMEs.

For (3), this would mean .neutrinorc.js is no longer a source of truth, more clutter in the root of projects even when using defaults, and another breaking change for people upgrading from Neutrino 8.

I think we should do either (1) or (2).

@timkelty
Copy link
Contributor Author

@edmorley I'm in favor of 2.

I've run into problems where I was doing the require('browserslist')() thing, and thus was requiring browserslist in my package.json, but the browserslist that babel-preset-env uses was different – ending up with a Chrome 69 is not a valid version or something…this is remedied by just not requiring your own browerslist dep, but still a potential sticking point.

@edmorley
Copy link
Member

I've run into problems where I was doing the require('browserslist')() thing, and thus was requiring browserslist in my package.json, but the browserslist that babel-preset-env uses was different

With the changes above to make (1) easier, it wouldn't be necessary to have a direct dependency on browserslist, so that issue wouldn't occur. Was that the main reason for you preferring (2)?

@timkelty
Copy link
Contributor Author

timkelty commented Sep 26, 2018

@edmorley with 1), are you suggesting we just tell users to do what I'm already doing? require('browserslist')(), and passing to targets?

@edmorley
Copy link
Member

No, this:

For (1), we could make this simpler to do, by supporting passing targets: false, that way Babel's default behaviour kicks in, and no need for the manual require('browserslist')().

ie:

// .neutrinorc.js
module.exports = {
  options: {
    root: __dirname,
  },
  use: [
    [
      '@neutrinojs/web',
      {
        // Stop Neutrino from setting targets, so @babel/preset-env uses browserlist instead.
        targets: false,
      }
    ]
  ]
};

@timkelty
Copy link
Contributor Author

🤦‍♀️ my bad, in that case 1) is my vote!

@timkelty
Copy link
Contributor Author

Think we should do this directly on options.targets, or options.targets.browsers, or both?

    [
      '@neutrinojs/web',
      {
        // Stop Neutrino from setting targets, so @babel/preset-env uses browserlist instead.
        targets: {
          browsers: false,  
        },
      }
    ]

@timkelty
Copy link
Contributor Author

Just noticed this: https://babeljs.io/docs/en/babel-preset-env#targetsbrowsers

Note: this will be removed in later version in favor of just setting "targets" to a query directly.

@edmorley
Copy link
Member

Another consideration is babel/babel-loader#690.

@timkelty
Copy link
Contributor Author

@edmorley I'm so glad you mentioned babel/babel-loader#690, I had to set cacheDirectory: false when testing this or I would have been very confused 😛

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants