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 emit utilities containing invalid theme fn keys #9319

Merged
merged 2 commits into from
Sep 14, 2022

Conversation

thecrypticace
Copy link
Contributor

Fixes #9318

This fix isn't ideal — need to iterate on it some before merging. We shouldn't even generate the utility in the first place. The problem here is the utility and the evaluation of the theme() function are at two separate stages. This causes the utility to be stored in the map when it is otherwise technically invalid. As a perf optimization we don't clear out the utility list when "watching" — it's effectively append-only causing this to throw as soon as you accidentally introduce a utility w/ a theme key that doesn't exist.

@thecrypticace
Copy link
Contributor Author

Hey @RobinMalfait how bad of an idea is this — or does this seem fine-ish?

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.

Hmm, I see what you mean. Instead of silently ignoring I would expect an error/warning being thrown instead so that you have at least something to debug (or open an issue about).

I think in the past we've handled the resolving of the theme function with special cases for some plugins. Ideally we figure out a way to resolve the theme value value per-plugin but that's probably out of scope for this one.

I think what we can do here is:

  1. Throw some form of error in the general case when theme function doesn't resolve to a proper (string?) value.
  2. Handle the boxShadow properly if that's not the case already (doubt it).

I think we've used throw node.error('Message') before which should be recoverable (the process should not crash but an error should be shown in the terminal)

@thecrypticace thecrypticace marked this pull request as ready for review September 14, 2022 17:06
@thecrypticace thecrypticace force-pushed the fix/no-invalid-theme-fn-utilities branch from db92895 to 5ab4414 Compare September 14, 2022 17:22
@thecrypticace thecrypticace changed the title Don't emit generated utilities with invalid uses of theme functions Don't emit utilities containing invalid theme fn keys Sep 14, 2022
@thecrypticace thecrypticace merged commit 8fe6f48 into master Sep 14, 2022
@thecrypticace thecrypticace deleted the fix/no-invalid-theme-fn-utilities branch September 14, 2022 17:27
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.

Tailwind compiler crashes when you try to [--box-shadow:theme('boxShadow.doesNotExist')]
2 participants