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

Migrate @media screen(…) #14603

Merged
merged 6 commits into from
Oct 10, 2024
Merged

Migrate @media screen(…) #14603

merged 6 commits into from
Oct 10, 2024

Conversation

RobinMalfait
Copy link
Member

This PR adds a codemod that migrates the @media screen(…) to the properly expanded @media (…) syntax.

@media screen(md) {
  .foo {
    color: red;
  }
}

Will be converted to:

@media (width >= 48rem) {
  .foo {
    color: red;
  }
}

If you happen to have custom screens (even complex ones), the screen will be converted to a custom media query.

@media screen(foo) {
  .foo {
    color: red;
  }
}

With a custom tailwind.config.js file with a config like this:

module.exports = {
  // …
  theme: {
    screens: {
      foo: { min: '100px', max: '123px' },
    },
  }
}

Then the codemod will convert it to:

@media (123px >= width >= 100px) {
  .foo {
    color: red;
  }
}

@RobinMalfait RobinMalfait changed the title Migrate screen(…) Migrate @media screen(…) Oct 4, 2024
Copy link
Contributor

@thecrypticace thecrypticace left a comment

Choose a reason for hiding this comment

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

💪

Copy link
Member

@adamwathan adamwathan left a comment

Choose a reason for hiding this comment

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

Nice! I think this should use the theme function instead of inlining the raw value though.

@RobinMalfait RobinMalfait force-pushed the feat/screen-codemods branch 2 times, most recently from 81b4998 to 28b2c1a Compare October 7, 2024 10:49
@RobinMalfait
Copy link
Member Author

Made some updates:

  1. If we can use the theme(--breakpoint-{name}) function, then we will use that:
    it('should migrate a built-in breakpoint', async () => {
    expect(
    await migrate(css`
    @media screen(md) {
    .foo {
    color: red;
    }
    }
    `),
    ).toMatchInlineSnapshot(`
    "@media (width >= theme(--breakpoint-md)) {
    .foo {
    color: red;
    }
    }"
    `)
    })
  2. If we cannot, then we will expand the full media query.
    it('should migrate a raw media query', async () => {
    expect(
    await migrate(
    css`
    @media screen(foo) {
    .foo {
    color: red;
    }
    }
    `,
    {
    theme: {
    screens: {
    foo: { raw: 'only screen and (min-width: 123px)' },
    },
    },
    },
    ),
    ).toMatchInlineSnapshot(`
    "@media only screen and (min-width: 123px) {
    .foo {
    color: red;
    }
    }"
    `)
    })

Also tried with an alternative using @custom-media, but since we can't use that I reverted it. You can see the changes here: a1113f8

@RobinMalfait RobinMalfait force-pushed the feat/screen-codemods branch 4 times, most recently from 232e078 to 8d1576d Compare October 10, 2024 11:37
In other cases, or using legacy JS object syntax, we can't use the
`theme(…)` function, therefore we fully expand the media query in your
CSS.
This reverts commit a1113f8.

This is way too experimental and no browser implements this feature yet.
See: https://caniuse.com/?search=custom-media

We can use lightningcss that transpiles is properly, but in environments
such as the CDN we don't have access to lightningcss.
@RobinMalfait RobinMalfait enabled auto-merge (squash) October 10, 2024 12:57
@RobinMalfait RobinMalfait dismissed adamwathan’s stale review October 10, 2024 13:06

Made sure that we use theme(…) in the simple cases and expand to the full @media if we can't use theme(…)

@RobinMalfait RobinMalfait merged commit 3ae22f1 into next Oct 10, 2024
1 check passed
@RobinMalfait RobinMalfait deleted the feat/screen-codemods branch October 10, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants