targets.js

[amk221]

My understanding was that ember-cli-autoprefixer took the list of browsers from targets.js. As of 1 beta, the suggestion is to use .browserlistrc.

I quite liked not having to maintain the list of browsers in multiple places.

What is the plan going forward? Is the intention that ember-cli will read from .browserlistrc too?

[amk221]

Also, what do we do about a conditional list

// taken from targets.js
if (isCI || isProduction) {
  browsers.push('last 2 Edge versions');
  browsers.push('ie 11');
}

[amk221]

Anybody got any input on this?

[bradleypriest]

Hi @amk221, I also brought this up in #45 (comment)

Sounds like Martin is definitely open to README improvements, and possibly to re-adding support

[amk221]

...so this means I end up with vendor prefixes for 'last 2 Edge versions' and 'ie 11' in my development builds. Prior to version 1 of ember-cli-autoprefixer, this wasn't the case. The point of the conditional in targets.js is for quicker build times in development. Can we get some insight into why this change was made?

[snewcomer]

@amk221 Can the logic you need be moved to ember-cli-build.js with overrideBrowserlist?

https://github.com/kimroen/ember-cli-autoprefixer/pull/45/files#diff-04c6e90faac2675aa89e2176d2eec7d8R21

[amk221]

Not sure how that would work? A .browserslistrc.js would do the trick, but that’s not a thing.

Edit: you changed the comment - oh I see - it could. But it doesn’t really round the issue off very well.

[snewcomer]

See overrideBrowserslist.

https://github.com/kimroen/ember-cli-autoprefixer/pull/45/files#diff-04c6e90faac2675aa89e2176d2eec7d8R21

[snewcomer]

Actually I see the issue. This wrapper library should feed the browser list in targets.js. One sec. Let me push a PR up.

[snewcomer]

@amk221 How does #51 look to you? @mfeckie Do you have any thoughts?

[amk221]

Seems good. I'm questioning why this was removed in the first place :)

[mfeckie]

@amk221 @snewcomer TL:DR -> I'm not against re-adding support for targets.js, but there are some things worth considering.

It was removed to bring the usage in line with how the underlying autoprefixer library expected to receive config. There's a conversation worth having about how these things should be handled in an Ember specific context. For example, the documentation for configuring autoprefixer expects the configuration to be read from different files This presents some possibility of confusion for someone reading the documentation for autoprefixer.

When I did the upgrade I referenced the advice in the autoprefixer docs

overrideBrowserslist (array): list of queries for target browsers. Try to not use it. The best practice is to use .browserslistrc config or browserslist key in package.json to share target browsers with Babel, ESLint and 

I can see the value of re-using the targets config from a consistency perspective, however, I don't think that this supports the full range of configuration options (https://github.com/postcss/autoprefixer#using-environment-variables-to-support-css-grid-prefixes-in-create-react-app for example, shows using different values for development vs production)

There's an argument to be made that this should be a major version bump. Anyone using the 1.x version would have their current configuration living in .browserslistrc or browserslist key in package.json, as I believe this change would mean they would be ignored.

[snewcomer]

I considered the Node 10 a major. The fact that config/targets.js is not passed to autoprefixer can be considered a bug.

Is your desire to pass in two different browser lists to Babel and autoprefixer? I haven't come across an app where these two configurations were different (yet :) ).

[mfeckie]

I'm not concerned about moving back to targets.js, I'm concerned that anyone using the current 1.x version would have any config they put in .browserslistrc or in package.json ignored if that PR gets merged. That seems like a 'breaking' level change.

[snewcomer]

Agreed. I think we can muster up something...order of importance -

1. has browserlistrc file - no overrideBrowserslist
2. has package.json browserslist config - no overrideBrowserslist
3. assign config/targets to overrideBrowserslist.

What do you think?

[mfeckie]

Sounds reasonable

[amk221]

Ok, thanks for the info.

I suppose the inverse might be nice... I mean, that ember-cli didn't use targets.js, but just consumed .browserlistrc.

But that aside, I'm happy to go with whatever. Keeping it how it is now, or going with your new solution.

[snewcomer]

@amk221 @mfeckie Mind reviewing this PR? Thank you!!

https://github.com/kimroen/ember-cli-autoprefixer/pull/51/files#diff-168726dbe96b3ce427e7fedce31bb0bcR29

[snewcomer]

1.0.1 released. Lmk if you have any issues!