Skip to content

Conversation

@thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Dec 4, 2024

This adds an API that lets us provide @theme variable completions from within core. The old list was hardcoded in IntelliSense but since the names for some of these changed recently we need a way to control this from within Tailwind CSS itself.

IntelliSense PR: tailwindlabs/tailwindcss-intellisense#1104

Do we think this is worth a changelog entry? Kinda thinking not but idk

@thecrypticace thecrypticace marked this pull request as ready for review December 4, 2024 03:29
@thecrypticace thecrypticace requested a review from a team as a code owner December 4, 2024 03:29
@philipp-spiess
Copy link
Member

@thecrypticace The way this is implement now, were just moving a hardcoded list from the intellisense repo to core but the list is still hardcoded and will likely go out of sync easily, or am I missing something here? 🤔

Do we think this is worth a changelog entry? Kinda thinking not but idk

I'd say if it fixes an issue that users are seeing then yeah but to my understanding this won't do anything unless you also update intellisense?

@thecrypticace
Copy link
Contributor Author

thecrypticace commented Dec 4, 2024

Yes we're moving the list into core even if it's hardcoded right now. So while we could miss something we at least have an avenue to update it in a new beta release. I did update some of the entries (we should double check to see which ones we want to include / change / whatever).

I'd say if it fixes an issue that users are seeing then yeah but to my understanding this won't do anything unless you also update intellisense?

That is correct.

@philipp-spiess
Copy link
Member

@thecrypticace Forgive me stupidity but it's unclear what problem this is really solving? I don't think we're currently planning on making any more changes to the variables and adding an allow list in core just because it might seems a bit much especially since it's still fixable by updating the intellisense extension anyways? Since this is referenced in the DesignSystem, all those strings are now always inlined in every build making potential future CDN builds bigger as well, for example. Not to mention the higher API surface.

@thecrypticace
Copy link
Contributor Author

thecrypticace commented Dec 4, 2024

I can just update the hardcoded list in IntelliSense instead if we want to. But the goal was to allow control from core. Whether we're going to update the list or not (I could see us adding entries to the list because what we have right now was cherry picked based on the default theme).

The API would in theory allow us to tailor the suggestions based on the current state of the theme in the future. For example, if we wanted to suggest --accent-color- because the theme is already using that variable we could.

I would like to, as much as possible, eliminate hardcoded data from IntelliSense.

@philipp-spiess
Copy link
Member

@thecrypticace Gotcha, yeah, I mean it's nice not to have to maintain a hardcoded list in a separate package for sure. I wonder if there's something we can do now that we maintain a list of allowed namespaces. Remember the place where we look up things like --font-* but have to ignore --font-size-* etc? I wonder if with this list, we can actually simplify this code so we don't have to manually mark some namespaces as ignored by others!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants