Skip to content
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

Disable Autoprefixer Grid #11453

Merged
merged 1 commit into from
Sep 26, 2018
Merged

Disable Autoprefixer Grid #11453

merged 1 commit into from
Sep 26, 2018

Conversation

ai
Copy link
Contributor

@ai ai commented Jul 4, 2018

@hansl thanks for being so brave to enable Grid feature in Autoprefixer.

Unfortunately, it may not be the best idea to enable it for everyone. This feature has a lot of limits. And must be explicitly controlled by a developer.

For instance, users complain about Grid warnings: postcss/autoprefixer#1062 (comment)

Maybe we can add an extra option to Angular CLI to enable Grid feature?

/cc @clydin

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@ai
Copy link
Contributor Author

ai commented Jul 4, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Jul 4, 2018
@tfalvo
Copy link

tfalvo commented Jul 5, 2018

Sorry but I don't understand, why we should disable completely grid feature in autoprefixer.
For example, in my case, I use grid layout and it was working perfectly with Angular.
But what about update you made in project mentioned at the beginning of this discussion.
Please @yepninja could you explain what is this change "IE doesn't support auto-fill and auto-fit. Let's don't add a prefixed prop and create warnings for these values.". And so why theses warnings appear even if you add browser rules omission on autoprefixer.

Is it possible to "unscope" IE but not all the grid feature ?

Sorry in advance if my comprehension is not good. but i'm a little afraid we'll shut down the tree while we can probably just cut a branch... ;-)

@ai
Copy link
Contributor Author

ai commented Jul 5, 2018

We don't need to disable it completely. Current idea is to disable it by default and enable only if you understand it's limits.

But I don't know Angular CLI enough to suggest a option to enable/disable Grid on the flight.

@clydin
Copy link
Member

clydin commented Jul 5, 2018

Unfortunately, disabling (permanently or by default) would be a breaking change. Also, the default browserslist file for a new project (found here) does not include IE9-11. Are there issues beyond IE support that need to be considered?

@ai
Copy link
Contributor Author

ai commented Jul 6, 2018

@clydin maybe we should give API to at least disable it?

What is user support IE 11 but use different polyfill for Grid?

@clydin
Copy link
Member

clydin commented Jul 11, 2018

Making it configurable is a potential option. I'll bring it up for discussion.

Also, I only found two alternate polyfills for IE. Both appear to be based on outdated specs and are runtime based. The second of which would make them impractical for use with Angular AOT as the component stylesheets are injected/removed at runtime. It appears that the autoprefixer approach is the most practical for Angular.

@tomschreck
Copy link

I found this workaround: #5964 (comment). The workaround has the comments above each item in CSS block. I found I only had to insert the comment once in each block.

@hansl
Copy link
Contributor

hansl commented Sep 25, 2018

I'm tempted to merge this PR, unless @clydin has some reservations. I'll merge this tomorrow morning if there's no objection.

@hansl hansl merged commit 5c6c704 into angular:master Sep 26, 2018
@mjgoethe
Copy link

mjgoethe commented Oct 24, 2018

@hansl Could this be made configurable? It's definitely an important feature for some. Alternatively, one possible workaround would be the grid control comment support that was added in autoprefixer 9.2, but Angular CLI is currently on 9.1.5.

I can PR one or both of these changes.

@garethrpratt
Copy link

@hansl as I understand it, the "grid: true" was originally introduced so that upgrading autoprefixer within angular-build wasn't a breaking change (as it used to default to true):
#7271 (comment)

We were also making use of this functionality, it would be useful to have a mechanism to turn it back on if possible.

@cedrickring
Copy link

cedrickring commented Nov 9, 2018

Would love to get this option back, as I don't want to prefix all my grid styles explicitly. (Took me ~3 hours to find out that it got disabled several weeks ago... and it's not even documented anywhere).

EDIT: For some reason adding /* autoprefixer grid: on */ to my .scss files doesn't work. Would be fine for me if that would be the option to enable it.

@garethrpratt
Copy link

Looks like the fix has now been merged to master:
#12785

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants