-
Notifications
You must be signed in to change notification settings - Fork 309
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
RFC: Shared Constants (Media Queries) #238
Comments
how can I make this work? |
@alejandroyunes This is an RFC, i.e Request for Comments. It is a proposal that does not work yet. Our plan is to implement this in January. |
that would be nice, because this is not working neither: desktopNav: {
display: {
default: "block",
"@media (max-width: 900px)": "none"
},
},`
but it works on dev |
I was trying something similar in my project (but in a basic form). I created a export const SM = '@media (min-width: 600px)';
export const MD = '@media (min-width: 769px)';
export const LG = '@media (min-width: 992px)';
export const XL = '@media (min-width: 1200px)'; In a import { MD } from './breakpoints';
const styles = stylex.create({
base: {
position: 'fixed',
left: {
default: 10,
[MD]: 20,
}
}
}); However, when this is compiled during development, I receive the following error: When I declare the value of the constant What I don't understand is that the value of Having the |
Yes, values need to defined statically within the same file. Imported values are not "static" as they need to be imported. Why?: With the current architecture every file compilation can be cached. Only if a file changes do we need to re-compile it. If we added support for imports, we would need to invalidate the cache of all dependent files whenever a file changed. @alejandroyunes That is an ESLint error, but I'm not able to reproduce it locally. It works. Maybe the error is on a different line? |
Thank you @nmn for the clarification. I am just curious, are media queries defined statically within every component in large projects that currently use StyleX? Regarding the proposed feature, I suggest using the same terms used in CSS when adding features to StyleX. I mean, using When defining a I also suggest adding a stylex.defineGlobals({
body: {
padding: {
default: 20,
[sm]: 10
}
},
h1: {
fontSize: {
default: fontSizes.lg, // where fontSizes is a StyleX variable
[sm]: fontSizes.md
}
},
small: {
fontSize: fontSizes.xs
}
}); The above can be transformed and added to the top of the generated CSS file instead of having another CSS file to accomplish the same purpose. |
Thanks for your feedback about the Regarding global styles, we are, by design, not going to be adding support for something like that anytime soon. Global styles are, by definition, not encapsulated. Adding a function like that would enable any component, anywhere, to be able to define global styles causing all the problems we're trying to solve. Our official position is that you should use as few global styles as possible, and when you absolutely need them, as is the case with resets, you should use an actual CSS file for that. It's much easier to debug global styles when they live in a single CSS file than if they're all over the codebase. NOTE: We also recommend using StyleX styles to style |
Based on the examples in the proposal, the export const media = stylex.defineConsts({
sm: stylex.types.media('(min-width: 640px) and (max-width: 767px)'),
md: stylex.types.media('(min-width: 768px) and (max-width: 1023px'),
lg: stylex.types.media('(min-width: 1024px) and (max-width: 1279px)'),
xl: stylex.types.media('(min-width: 1280px) and (max-width: 1535px)'),
xxl: stylex.types.media('(min-width: 1536px)'),
landscape: stylex.types.media('screen and (min-width: 30em) and (orientation: landscape)'), // may need a better key name
portrait: stylex.types.media('(min-height: 680px), screen and (orientation: portrait)'), // may need a better key name
}); This will simply output the The export const features = stylex.defineConsts({
flexbox: stylex.types.supports('(display: flex)'),
}); Regarding global styles, I agree with the basic principles. I currently have a dedicated file for resets; but what I proposed is to set those global styles once while being able to consume the shareable values (defined using StyleX) like colors, font sizes, spacings...etc. for consistency. I may not want to create a dedicated component for elements like |
@edamascus The examples make sense. The use-case for Good point regarding the use of constants from StyleX for resets. Will keep thinking about that. |
Regarding global css, you could:
A worse alternative (due to the necessity to give a name) might be to allow setting a constant name to reusable CSS variables. That way, you could reference variables defined with stylex anywhere else (you could also check duplicate variables names during bundling)
Both of the above might be useful to add to stylex for different users. However, I think these two ideas should be moved to separate issues, should I create them? |
We're considering something on these lines anyway, so I'm more in favour of this solution at the moment. It will take a while before we can achieve this though. @o-alexandrov You're welcome to create an issue to discuss this further. |
Are there any way today where i can put my media queries in a seperate file: breakpoints.stylex.ts import { gridTokens } from "./twTokens.stylex";
export const MOBILE = `@media only screen and (max-width: ${gridTokens.mobile})`;
export const TABLET = `@media only screen and (max-width: ${gridTokens.tablet})`;
export const DESKTOP = `@media only screen and (max-width: ${gridTokens.desktop})`; and use without getting compile errors? import stylex from "@stylexjs/stylex";
import { MOBILE } from "../../stylex/tw/breakpoints.stylex";
export const heroNoImageStyle = stylex.create({
hero: {
margin: {
default: "192px 0 96px",
[MOBILE]: "144px 0 48px;"
}
},
heroInner: {
display: "flex",
alignItems: "center",
justifyContent: "center",
},
heroContent: {
width: "100%",
maxWidth: "650px",
padding: "0 20px",
textAlign: "center"
}
}); |
@pksorensen No, which is why this proposal exists. Today, you can ether export styles themselves (the result of calling Also variables can't be used within Media Queries. It's not supported in CSS. Variables can only be used values of style properties. OR You use types to keep media queries consistent across files: breakpoints.stylex.ts export type Sm = "@media (max-width: 768px)";
export type Md = "@media (min-width: 768px) and (max-width: 1260px)";
export type Lg = "@media (min-width: 1260px)"; and use without getting compile errors? import stylex from "@stylexjs/stylex";
import type {Sm, Md, Lg} from "../../stylex/tw/breakpoints.stylex";
export type MOBILE: Sm = "@media (max-width: 768px)";
export type TABLET: Md = "@media (min-width: 768px) and (max-width: 1260px)";
export type DESKTOP: Lg = "@media (min-width: 1260px)";
export const heroNoImageStyle = stylex.create({
hero: {
marginTop: {
default: 192,
[MOBILE]: 144,
},
marginBottom: {
default: 96,
[MOBILE]: 48,
},
},
heroInner: {
display: "flex",
alignItems: "center",
justifyContent: "center",
},
heroContent: {
width: "100%",
maxWidth: "650px",
padding: "0 20px",
textAlign: "center"
}
}); |
Good sugestions, and thanks for pointing out the thing with variables in media queries. new to stylex, is there any historical data to say anything about timeframe that a RFC would be implemented? I took an alternative route for now to keep code clean and not having to write the queries all over and wrote a webpack plugin / loader to find and replace the imports before stylex babel plugin gets the code, so i have a working solution for now that allows me to do what i wrote above. |
This is another approach, but I don't recommend it as it hurts cache-ability. It's the most powerful though. Timeline: A month or two. Depends on how many smaller bugs come up. |
Just learning , babel, webpack is a jungle :D But if RFC is going as example above, i can simply remove my plugin when its implemented and change that one file without having to change anything else, so i can live with some slower builds until, or until i get to many bugs in my plugin. Going with types and such, i have to refactor more when RFC is done. Might change my mind when i get more experience with it. I was not able to gasp/change the stylex plugin , so thats why i just altered the source in webpack before it goes into stylex. |
@pksorensen For sure. It makes sense for your use-case. Just explaining why I don't suggest it pre-emptively. |
Just to confirm, would An example case is a library wanting to allow downstream users to modify a media query used in shipped components. |
No. variables created with What you're asking for is changing the value of a constant globally itself. This is a side-effect by definition and breaks encapsulation. As such, although I understand the use-case, we are not going to prioritise solving this use-case in the short term. |
This would be incredibly valuable. I'm currently creating a design system with stylex. Not being share these kind of values is posing challenges. Happy to help in any way I can to move this along. |
@enmanuel-lab49 This work has a couple of pre-requisites, namely integrating a CSS post-processing step. Once that is done, this work will be relatively trivial to implement. |
Loving this! I also love that you guys are thinking about things like Another note that I probably, because I already know my css, I would use |
@tounsoo We don't intend to choose one. Instead our plan is to create media queries that don't conflict by defining both @media (min-width: 768px) and (max-width: 1368px) { ... } This is why However these little details is why this RFC is still open and not implemented. We value any feedback on this proposal. |
@nmn What’s the cost benefit in the enforcing the entire range? If a developer ships a set of components that are mobile first and another developer ships components that define explicit ranges both will work in any StyleX project, no? It’s also the most common pattern to use One potential issue I see is poorly defined ranges (e.g. |
This is the core design question here. Since we're dealing with variables here, it's impossible to know the actual value of the media query while compiling the Enforcing one of Another easy solution would be to make this configurable, but that breaks our core principle of avoiding global configuration. We want StyleX to work the way for all apps. Media queries both defined locally and from this proposed API still have some potential opportunities of conflict. I'll continue to think about solving these issues, but I also want to ensure this RFC doesn't get stuck in limbo because of these edge-cases. My goal for now is to decide on the best API. If it's the right API design, we can work on the implementation details later. |
I am using a pattern for defining color variables using the const colors = stylex.defineVars({
background: "0 0 0%"
}) Then use this color variable by wrapping it in const styles = stylex.create({
// 50% opacity
backgroundColor: `hsl(${colors.background} / 0.5)`,
// 100% opacity
backgroundColor: `hsl(${colors.foreground})`,
// Mistake!
backgroundColor: colors.foreground,
}) but I often forget to wrap the color variable in im thinking the types feature can include a way to make this pattern safer. the proposed im thinking |
Could this be implemented as a lint in the eslint plug? |
@aspizu No because the variables are defined in one file but used in another. The Babel plugin or the ESLint plugin can't know what was stored within the variable. |
My exact instinct was to do just this and what @edamascus tried. I, like them also couldn't get it to work which is what led me here :) I'm only able to get it to work by declaring the I saw an option suggested above was to use the stylex.create but that doesn't solve my use case as I only want to re-use the media query as a const. I also couldn't get the
|
No. There is no plan to support this. This decision has been made after a lot of careful consideration and is essential for predictable style composition on both web and compatibility with inline styles on RN.
Yes, as long as the values are wrapped with the valid type. |
Moving to Discussions |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Motivation
The immediate motivation is to enable defining and exporting Media Queries the same way that variables can be shared using
defineVars
. In the future, this feature could be expanded to support additional share-able values.Inspiration
From StyleX itself, the idea is to expand
defineVars
API to able to share more kinds of values. Further, we’re adding helper functions such asstylex.types.length
andstylex.types.color
which will be used to mark values of variables with a particular type. This will not only add additional type-safety to the variable types, but also insert@property
CSS rules which gives those variables types in CSS itself, which enables certain unique features, such as being able to animate variables.From CSS, there is an old proposal for
@custom-media
that hasn’t been implemented in any browser.stylex.types
explainerWe are already working on utility functions to lock down the types of particular variable values. So, when using defineVars you can assign a type to a particular variable.
Here, the value
’black’
will be validated by the compiler and a compiler error will be thrown if a non-valid color is passed in. It also changes the behaviour of the variable generated. An @Property CSS rule will be generated that markscolors.primary
as a<color>
in CSS itself:This makes the variable itself animateable in CSS using
transition
oranimation
.The static types of the variable would be affected as well, and you’d be forced to use the same function to define values within
createTheme
.We can also consider adding utility functions like
stylex.types.rgb(0, 0, 0)
in the future. As all of these functions are compiled away, we can ensure that tree-shaking removes all these functions from the JS bundle.Proposal
The core proposal is to add special type for
atRules
tostylex.types
. I.e a new helper function,stylex.types.atRule
. We can also add convenience functions forstylex.types.media
andstylex.types.supports
if it makes sense.However, unlike CSS variables, the value of a custom at-rule needs to remain constant and cannot be overridden in a theme. This conflicts with the way
defineVars
andcreateTheme
work as a pair today. And so the proposal also includes a new function calleddefineConsts
. This new function will work exactly likedefineVars
except the variables created with it cannot be overridden withcreateTheme
. Additionally, certain types, like atRule will only be accepted withindefineConsts
and notdefineVars
.Example
Using it would be the same as using variables. You import and use the value.
Implementation Details
The implementation would be almost identical to how variables already work. The
defineConsts
call would output a variable with a media query value to the generated styles, and while generating the CSS file by combining all the collected styles, the media queries variables would be inlined with the actual value.This same process can be generalized to variables in general where any variable that is never overridden can be inlined directly and any unused variable can be removed.
Optional Extras
As mentioned earlier, we can add some additional utilities to make things easier, namely:
stylex.types.media
stylex.types.mediaWidth
stylex.types.mediaHeight
stylex.types.supports
Here’s what the example above could look like:
The main benefit of these convenience functions is reducing boilerplate.
The text was updated successfully, but these errors were encountered: