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

Move CSS env()/var() functions into css.types.* #25645

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

caugner
Copy link
Contributor

@caugner caugner commented Jan 13, 2025

Summary

Moves env() from css.properties.custom-property to css.types.

Test results and supporting details

Related issues

Fixes #14668.

@caugner caugner requested a review from Elchi3 January 13, 2025 22:14
@github-actions github-actions bot added data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS size:l [PR only] 101-1000 LoC changed labels Jan 13, 2025
Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should also move var() if we're moving env(), WDYT?

@queengooborg queengooborg added the needs collector update This PR needs a corresponding update to openwebdocs/mdn-bcd-collector. label Feb 13, 2025
@caugner
Copy link
Contributor Author

caugner commented Feb 18, 2025

I think that we should also move var() if we're moving env(), WDYT?

Good point, that probably makes sense.

PS: Wouldn't it also make sense to introduce css.functions.*? It seems weird that we mix CSS functions and CSS data types under css.types.*.

@caugner caugner force-pushed the 14668-css-env-function branch from 307db11 to e6de5ae Compare February 18, 2025 16:05
@caugner caugner changed the title Move CSS env() function into css.types.* Move CSS env()/var() functions into css.types.* Feb 18, 2025
@caugner caugner requested review from queengooborg and removed request for ddbeck and Elchi3 February 18, 2025 16:06
@caugner
Copy link
Contributor Author

caugner commented Feb 21, 2025

@Elchi3 Please let me know if you have any concerns here.

@ddbeck
Copy link
Collaborator

ddbeck commented Feb 22, 2025

I'm fine with this PR.

PS: Wouldn't it also make sense to introduce css.functions.*? It seems weird that we mix CSS functions and CSS data types under css.types.*.

Previously:

No, don't do this. Types are a bit dubious to begin with and giving functions their own namespace would compound the error. It would also trigger a lot of needless work for consumers.

Types are a convenience for specification authors, much like IDL mixins. Browsers don't actually have to implement the specified type system, just the properties and descriptors that consume concrete values that correspond to their specified type. There are many examples of supported BCD "types" that only work incompletely across all the properties spec'd to use that type or whole types that are partially implemented (#18198). It's messy and fixing it would require significant work, comparable to flattening mixins (#8929).

Besides, a lot of CSS functions are a functional notation for a type (e.g., translateX() or url()). With the exception of a few oddities like var(), there are few composable functions in CSS as a language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS needs collector update This PR needs a corresponding update to openwebdocs/mdn-bcd-collector. size:l [PR only] 101-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

css.properties.custom-property - does env() belong here?
3 participants