RFC: Shared Constants (Media Queries)

nmn commented 6 months ago

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 as stylex.types.length and stylex.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` explainer

We 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. ```tsx import * as stylex from '@stylexjs/stylex'; export const colors = stylex.defineVars({ primary: stylex.types.color('black'), // ... }); ``` 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 marks `colors.primary` as a `` in CSS itself: ```css @propert --colors-primary { syntax: ''; inherits: true; initial-value: black; } ``` This makes the variable itself animateable in CSS using `transition` or `animation`. 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`. ```ts const dracula = stylex.createTheme(colors, { // ERROR: Expected a type.Color, got a string. primary: 'purple', // OK! primary: stylex.types.color('purple'), }); ``` 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 to stylex.types. I.e a new helper function, stylex.types.atRule. We can also add convenience functions for stylex.types.media and stylex.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 and createTheme work as a pair today. And so the proposal also includes a new function called defineConsts. This new function will work exactly like defineVars except the variables created with it cannot be overridden with createTheme. Additionally, certain types, like atRule will only be accepted within defineConsts and not defineVars.

Example

// globalTokens.stylex.js
import * as stylex from '@stylexjs/stylex';

export const media = stylex.defineConsts({
  sm: stylex.types.atRule('@media (min-width: 640px) and (max-width: 767px)'),
  md: stylex.types.atRule('@media (min-width: 768px) and (max-width: 1023px'),
  lg: stylex.types.atRule('@media (min-width: 1024px) and (max-width: 1279px)'),
  xl: stylex.types.atRule('@media (min-width: 1280px) and (max-width: 1535px)'),
  xxl: stylex.types.atRule('@media (min-width: 1536px)'),
});

export const colors = stylex.defineVars({
  primary: stylex.types.color('black'),
  // ...
});

Using it would be the same as using variables. You import and use the value.

import * as stylex from '@stylexjs/stylex';
import {media, colors} from './globalTokens.stylex'

const styles = stylex.create({
  base: {
    width: {
      default: '100%',
      [media.sm]: 500,
      [media.md]: 800,
      [media.lg]: 960,
    },
    color: colors.primary,
  },
});

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:

import * as stylex from '@stylexjs/stylex';

export const media = stylex.defineConsts({
  sm: stylex.types.mediaWidth(640, 768),
  md: stylex.types.mediaWidth(768, 1024),
  lg: stylex.types.mediaWidth(1024, 1280),
  xl: stylex.types.mediaWidth(1280, 1536),
  xxl: stylex.types.media('(min-width: 1536px)'),
});

The main benefit of these convenience functions is reducing boilerplate.

alejandroyunes commented 6 months ago

how can I make this work?

nmn commented 6 months ago

@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.

alejandroyunes commented 6 months ago

that would be nice, because this is not working neither:

  desktopNav: {
    display: {
      default: "block",
      "@media (max-width: 900px)": "none"
    },
  },`

Error: This is not a key that is allowed by stylex @stylexjs/valid-styles

but it works on dev

edamascus commented 6 months ago

I was trying something similar in my project (but in a basic form). I created a breakpoints.ts file with the following exported constants:

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 Component.tsx file I imported the constant and attempted to use it as follows:

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: Only static values are allowed inside of a stylex.create() call.

When I declare the value of the constant MD within the Component.tsx file the same code works without any issues.

What I don't understand is that the value of MD is indeed static and I am not sure why the computed key is not working when the variable used to define it is imported.

Having the defineConst() function would be very useful!

nmn commented 6 months ago

When I declare the value of the constant MD within the Component.tsx file the same code works without any issues.

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?

edamascus commented 6 months ago

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 stylex.types.media() and stylex.types.supports makes more sense than stylex.types.atRule().

When defining a media rule, the same principles in CSS should be followed without the need of creating additional layers like mediaWidth and mediaHeight. This keeps the rule flexible and in coherence with CSS.

I also suggest adding a defineGlobals() API with the purpose of defining global CSS rules that might depend on the variables and media rules which are defined by StyleX and can also be used to set CSS resets. For example:

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.

nmn commented 6 months ago

Thanks for your feedback about the media and supports function names. Please provide some examples of the usage of those functions that would feel most natural.

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 html and body elements when possible.

edamascus commented 6 months ago

Based on the examples in the proposal, the media() function may be used as follows:

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 @media query followed by whatever is passed to the media() function. There can be many combinations and trying to create a utility for each of them like and and not and width(min, max) might be too much. From what I understand, the purpose here is to share the value and it is not about how those values are declared.

The supports() function can be used in a similar manner:

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 h1, h2, small... and consuming the variables will not be possible.

nmn commented 6 months ago

@edamascus The examples make sense. The use-case for mediaWidth and mediaHeight functions would be to enforce min-width and max-width for media queries. In a project with lots of media queries, the only way to avoid conflicts is by defining both bounds.

Good point regarding the use of constants from StyleX for resets. Will keep thinking about that.

o-alexandrov commented 6 months ago

Regarding global css, you could:

add stylex API for it
add an option for the bundling plugin to enforce that such API is called only once per project; otherwise, it triggers a bundle error

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)

this, btw, might address current concerns of material ui about allowing to combine stylex with other libraries in a project https://github.com/mui/material-ui/issues/34826#issuecomment-1842933183

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?

nmn commented 6 months ago

A worse alternative might be to allow setting a constant name to reusable CSS variables.

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.

pksorensen commented 6 months ago

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"
        }
    });

nmn commented 6 months ago

@pksorensen No, which is why this proposal exists.

Today, you can ether export styles themselves (the result of calling stylex.create).

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"
    }
});

pksorensen commented 6 months ago

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.

nmn commented 6 months ago

wrote a webpack plugin / loader to find and replace the imports before stylex babel plugin

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.

pksorensen commented 6 months ago

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.

nmn commented 6 months ago

@pksorensen For sure. It makes sense for your use-case. Just explaining why I don't suggest it pre-emptively.

joshuabaker commented 5 months ago

Just to confirm, would stylex.types.mediaWidth (et al.) also work with defineVars?

An example case is a library wanting to allow downstream users to modify a media query used in shipped components.

nmn commented 5 months ago

Just to confirm, would stylex.types.mediaWidth (et al.) also work with defineVars?

No. variables created with defineVars can be changed for particular UI sub-trees using createTheme. This is not possible with media queries.

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.

enmanuel-lab49 commented 5 months ago

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.

nmn commented 5 months ago

@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.

tounsoo commented 3 months ago

Loving this! I also love that you guys are thinking about things like stylex.types.mediaWidth(640, 768), but I think depending on the teams decision, it could be mobile first or desktop first that a user might need. Expanding mediaWidth to mediaMinWidth etc would probably be possible but I think once we start going that down that rabbit hole, that would be a deep one. I would be happy with stylex.types.atRule().

Another note that I probably, because I already know my css, I would use stylex.types.atRule() even if there was more option because it would just be easier for me to consume.

nmn commented 3 months ago

it could be mobile first or desktop first

@tounsoo We don't intend to choose one. Instead our plan is to create media queries that don't conflict by defining both min-width and max-width.

@media (min-width: 768px) and (max-width: 1368px) { ... }

This is why stylex.types.mediaWidth(640, 768) is described the way it is. It enforces that the developer passes in the entire range for a media query and not just one of them. And you'd be expected to pass (0, x) or (x, Infinity) for the edges.

However these little details is why this RFC is still open and not implemented. We value any feedback on this proposal.

joshuabaker commented 3 months ago

@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 min-width. I haven’t actually seen a system that approaches this differently. It avoids excessive CSS (e.g. display: grid at min-width: 768px, instead of having to add the display: grid into multiple media queries or adding an additional range).

One potential issue I see is poorly defined ranges (e.g. min-width: 768px and max-width: 1024px with a second min-width: 1024px and max-width: 1368px range). Yes, this is bad CSS, but the system allows it.

nmn commented 3 months ago

One potential issue I see is poorly defined ranges (e.g. min-width: 768px and max-width: 1024px with a second min-width: 1024px and max-width: 1368px range). Yes, this is bad CSS, but the system allows it.

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 stylex.create call.

Enforcing one of min-width or max-width queries globally is the easy solution here, but I'm hesitant to do that. It would make StyleX more opinionated about how the web should be designed. While "Mobile First" is a widely accepted design trend, it is still a trend that doesn't apply universally.

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.

aspizu commented 2 months ago

I am using a pattern for defining color variables using the hsl() css function.

const colors = stylex.defineVars({
  background: "0 0 0%"
})

Then use this color variable by wrapping it in hsl()

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 hsl(${...}) which breaks it.

im thinking the types feature can include a way to make this pattern safer.

the proposed types.color wouldn't allow putting hsl values without enclosing it in hsl().

im thinking types.hsl and types.rgb to support this pattern.

nmn commented 2 months ago

Try this:

const colors = stylex.defineVars({
  background: "0 0 0%" as `${number} ${number} ${number}%`
})

It won't validate correct usage, but should help catch the mistake early.

Adding types for such tuples doesn't solve the problem. The compiler doesn't know the type of the variable when it is being used. The only way to do the validation is to validate the generated CSS file. This doesn't depend on types.

aspizu commented 2 months ago

Could this be implemented as a lint in the eslint plug?

nmn commented 2 months ago

@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.

TeoAlmonte commented 1 month ago

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"
        }
    });

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 const in each file but that of course is not scalable const MOBILE = '@media (min-width: 768px)'

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 type option above to work with exported const vars either.

As for the proposal goes, would something like this work?

export const rules = stylex.defineConsts({
  sm: '@media (max-width: 768px)',
  md: '@media (max-width: 1024px)',
  lg: `@media (max-width: ${breakpoint.lg})`,
  dark: '@media (prefers-color-scheme: dark)',
  sGrid: '@supports (display: grid)'
});

Also... is there a plan to allow for top-level nested media queries like this? it would save a lot of time. I know there's a trade-off here in how the pattern here but just curious.

container: {
display: 'flex',
flexWrap: 'wrap',
flexDirection: 'column',
'@media (min-width: 768px)': {
  display: 'none',
  color: 'red'
},
'@media (min-width: 1024px)': {
  display: 'flex',
  color: 'blue'
}
},

nmn commented 1 month ago

Also... is there a plan to allow for top-level nested media queries like this? it would save a lot of time.

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.

As for the proposal goes, would something like this work?

Yes, as long as the values are wrapped with the valid type. stylex.types.media or similar.

facebook / stylex