Skip to content
This repository has been archived by the owner on May 11, 2018. It is now read-only.

Send file path to browserslist #26

Closed
ai opened this issue Oct 13, 2016 · 25 comments
Closed

Send file path to browserslist #26

ai opened this issue Oct 13, 2016 · 25 comments

Comments

@ai
Copy link

ai commented Oct 13, 2016

Browserslist could use browserslist config file. It is useful, because many different tools will share same config (Autoprefixer, Stylelint, cssnext, doiuse).

But to enable a config, you need to pass a path for current file:

browserslist(opts.browsers, { path: opts.file })
@hzoo
Copy link
Member

hzoo commented Oct 13, 2016

I guess we could check the file but given a preset could be somewhere else I'm not sure.

Also relevant facebook/create-react-app#892

@hzoo
Copy link
Member

hzoo commented Nov 10, 2016

If someone wants to make a PR

@Jessidhia
Copy link
Member

Jessidhia commented Nov 10, 2016

I tried looking into it, but the preset doesn't have access to useful information about the .babelrc or the file being processed 😕

The preset is called with a context argument, but it turns out that the context is actually just the node api namespace, which you can easily import yourself, instead of an actual context, so there's not even any way to know which .babelrc is being used, let alone which file is being transformed 😢

@p-jackson
Copy link

I made a PR, but having slept on it I feel like the original suggestion would be much more elegant.

Looks like it'd be possible to change babel-core to pass the current file to the preset. Do you think babel-core would be open to a PR like that?

@ai
Copy link
Author

ai commented Jan 4, 2017

@hzoo I changed Browserslist to fit Babel and CRA requirements. Now it is your turn to enable a config (not just a option).

@motiz88
Copy link

motiz88 commented Feb 19, 2017

Hey @ai, I'd like to jump in and get this working on the Babel side. I'm not sure what you mean by your last comment though - could you elaborate?

@ai
Copy link
Author

ai commented Feb 19, 2017

@motiz88 I mean that it is Babel turn to set path option in Browserslist to load config even if browsers option is missed in .babelrc.

@motiz88
Copy link

motiz88 commented Feb 19, 2017

babel/babel#4834 would enable this nicely.

Related: It's worth considering how this fits together with #161, which supports specifying an explicit path in the preset options. Possible behaviours off the top of my head:

  1. If an explicit path is set, never look up any per-file config via browserslist.
  2. If an explicit path is set, try it first; If a browserslist config can't be found that way, fall back to looking at each file's path.
  3. Lookup browserslist config relative to each file, and only if that fails - fall back to the explicit path (if any).

@ai
Copy link
Author

ai commented Feb 19, 2017

I think, that if user set config path, we should use only this config.

@yavorsky
Copy link
Member

yavorsky commented Feb 19, 2017

We already have #161. It accepts all logic provided by browserslist, including path. The only thing it makes extraordinary is set targets specified in preset-env options as default browsers for browserslist to prevent default browsers if we hadn't set browsers in targets. It's temporary and I don't think it could affect workflow somehow.

@motiz88
Copy link

motiz88 commented Feb 19, 2017

@yavorsky One thing #161 doesn't do (please correct me if I'm wrong) is allow babel-preset-env to pick up an existing Browserslist config file without requiring path to be set. This is what I'm getting at here, in keeping with the idea that

All tools that rely on Browserslist will find its config automatically [...in package.json or browserslist]

@yavorsky
Copy link
Member

yavorsky commented Feb 19, 2017

@motiz88 It handles path option - https://github.com/babel/babel-preset-env/pull/161/files#diff-1fdf421c05c1140f6d71444ea2b27638R119, so it's possible to set:

["env", {
  "path": "path/to/config"
}]

where path/to/config - directory with package.json or browserslist's config.
You can dive deeper in the fixtures.

Without path option it would start to search in process.cwd(). Also, it accepts BROWSERSLIST_CONFIG var.
Actually, we don't handle the behaviour related to config search. We only process and pass correct arguments to the browserslist (browsers, path, defaults) and let it do the rest of job.

This PR isn't merged yet, so it's cool to see more propositions about technical realization, etc. 😉

@ai
Copy link
Author

ai commented Feb 19, 2017

@yavorsky in next major Browserslist will not look in process.cwd() on missed path.

path option in Browserslist should be set not to config, but to processed JS file.

@yavorsky
Copy link
Member

@ai But browserslist currently use path option only to find a config file?

If a query is missing, Browserslist will look for a config file. You can provide a path option (that can be a file) to find the config file relatively to it.

@ai
Copy link
Author

ai commented Feb 19, 2017

Yeap, but semantically path is a path to processed file (JS file for Babel or CSS for Autoprefixer).

So in Autoprefixer/Stylelint, user doesn’t need to specify path to config manually. Developer just processes files in Autoprefixer. And Browserslist automatically find config in CSS files parents directories.

@yavorsky
Copy link
Member

yavorsky commented Feb 19, 2017

But in the case of preset-env we haven't any js files on preset building stage. We only have possible process root and config provided by the user. 🤔
@ai What behavior will be if path wasn't specified? No config search at all?

@ai
Copy link
Author

ai commented Feb 19, 2017

@yavorsky as a short-term solution rename path option to config and reduce UX by asking user to always pass a config path.

As a long-term solution try to change API to allow rebuild preset for every file.

I think users will expect same behaviour as for .babelrc for browserslist.

@yavorsky
Copy link
Member

@ai Thanks! 👍 Also I'll play with preset's context and maybe it helps.

@motiz88
Copy link

motiz88 commented Feb 19, 2017

@ai @yavorsky Babel does rebuild the preset for every input file, and as I alluded to above, babel/babel#4834 amends that behaviour to pass the input path as an extra argument to the preset constructor. Hopefully that PR is on its way to being merged on the Babel side.

@yavorsky
Copy link
Member

yavorsky commented Feb 19, 2017

@motiz88 I mean it doesn't support path for babel.transform to use it from loaders/plugins with builders.

Thanks for the link, I really never met or just forgot about this PR. 🎉

@alexander-akait
Copy link
Contributor

@yavorsky What is the final implementation algorithm? I want to champion this PR 😄

@stale
Copy link

stale bot commented Jun 14, 2017

This issue has been automatically marked as stale because it has not had recent activity 😴. It will be closed if no further activity occurs. Thank you for your contributions 👌!

@knynkwl
Copy link

knynkwl commented Sep 2, 2017

Any updates on this?

@stevewillard
Copy link

@knynkwl I think 2.x version supports this feature, so you would need to upgrade to babel 7

@hzoo
Copy link
Member

hzoo commented Sep 2, 2017

Done in #161 (correct for v7)

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

No branches or pull requests

9 participants