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

Don't crash when setting JS theme value to null #16210

Merged
merged 4 commits into from
Feb 4, 2025

Conversation

philipp-spiess
Copy link
Member

Closes #16035

In v3 it was possible to unset a specific color namespace by setting doing something like this:

export default {
  theme: {
    extend: {
      colors: {
        red: null,
      },
    },
  },
}

This pattern would crash in v4 right now due to the theme access function not being able to work on the red property being a null. This PR fixes this crash.

However it leaves the behavior as-is for now so that the red namespace defined via CSS will still be accessible. This is technically different from v3 but fixing this would be more work as we only allow unsetting top-level namespaces in the interop layer (via the non-extend-theme-object). I would recommend migrating to the v4 API for doing these partial namespace resets if you want to get rid of the defaults in v4:

@theme {
  --color-red-*: initial;
}

Test plan

The crash was mainly captured via the test in compat/config.test.ts but I've added two more tests across the different levels of abstractions so that it's clear what null should be doing.

@philipp-spiess philipp-spiess requested a review from a team as a code owner February 3, 2025 15:29
@philipp-spiess philipp-spiess changed the title Don't crash when setting JS theme value to null' Don't crash when setting JS theme value to null Feb 3, 2025
Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

Small suggested change, otherwise looks good!

CHANGELOG.md Outdated Show resolved Hide resolved
@philipp-spiess philipp-spiess merged commit 3b61277 into main Feb 4, 2025
5 checks passed
@philipp-spiess philipp-spiess deleted the fix/null-in-js-theme branch February 4, 2025 14:43
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.

[v4] "Cannot read properties of null (reading '50')" when using theme.extend.colors.zinc: null
2 participants