tailwindlabs / tailwindcss-intellisense

Intelligent Tailwind CSS tooling for Visual Studio Code
2.75k stars 183 forks source link

Intellisene doesn't work when `)` is used #868

Closed bengry closed 3 months ago

bengry commented 8 months ago

What version of VS Code are you using?

For example: v1.84.0-insider

What version of Tailwind CSS IntelliSense are you using?

For example: v0.10.1

What version of Tailwind CSS are you using?

For example: v3.3.3

What package manager are you using?

For example: yarn

What operating system are you using?

For example: macOS

Tailwind config N/A

VS Code settings

"tailwindCSS.experimental.classRegex": [
      ["clsx\\(([^)]*)\\)"],
      ["cva\\(([^)]*)\\)", "[\"'`]([^\"'`]*).*?[\"'`]"],
      ["lintClassnames\\(([^)]*)\\)", "[\"'`]([^\"'`]*).*?[\"'`]"],
      ["class[Nn]ames\\(([^)]*)\\)", "(?:'|\"|`)([^']*)(?:'|\"|`)"],
      ["cx\\(([^)]*)\\)", "(?:'|\"|`)([^']*)(?:'|\"|`)"]
],
"tailwindCSS.classAttributes": ["class", "className", ".*Class(es)?"]

Reproduction URL

N/A

Describe your issue

When using classes such this call to cx:

const className = cx(
  'fixed left-0 top-0 z-10 translate-x-[calc(50vw-50%)] translate-y-[calc(50vh-50%)]'
);

or this call to cva:

const myComponent = cva({
  base: [
    '[--dialog-px:theme(spacing.6)] [--dialog-spacing:theme(spacing.4)]'
  ]
});

Intellisense doesn't work. I think this is because the regex for matching these calls relies on the assumption that ) won't be used inside them. If I break the first example like so for example:

const className = cx(
  'fixed left-0 top-0 z-10 translate-x-[calc(50vw-50%] translate-y-[calc(50vh-50%]'
);

Intellisense works inside again.

The issue is that regex can't be used to match nested expressions.

I think this extension needs a more robust way to handle calls such as cx, classNames, cva and others - one that's probably built-in and does the matching parentheses matching in a more robust way. I think a configuration like: "tailwindCSS.experimental.callees" makes sense:

// settings.json
{
  "tailwindCSS.experimental.callees": ["classNames", "cx", "cva"]
}

If string manipulation is still done, it can be assumed that the next character after the last letter in the "callee" is a (, and then matching it's ) is where the intellisense, hovers etc. all end.

keywords: tailwindCSS.experimental.classRegex, nested expressions, calc, theme

mskelton commented 8 months ago

You can solve this issue with a slightly modified regex. Basically, split the regex into two branches with any number of each branch. The first branch is any non-paranthesis and the second branch is a set of matches parenthesis with one or more characters inside of them.

["clsx\\((([^()]*|\\([^()]*\\))*)\\)"],

https://regex-vis.com/?r=clsx%5C%28%28%5B%5E%28%29%5D*%7C%5C%28%5B%5E%28%29%5D*%5C%29%29*%5C%29&e=0

bengry commented 8 months ago

Thanks @mskelton. Looks like it solves the first example I gave, but changing the setting to:

"tailwindCSS.experimental.classRegex": [
      ["cx\\((([^()]*|\\([^()]*\\))*)\\)"],
      ["cva\\((([^()]*|\\([^()]*\\))*)\\)"],
]

Does seem to sometimes work, but even when it does - it seems to hang up VS Code when trying to save files. I couldn't find an exact reproduction unfortunately, but reverting this to what I had before (and this is the only setting I changed) fixes the hangups while saving. I'm guessing it's the regex parser working hard, especially when it seems large (~100 LOC) cva expressions.

Lastly, even if this is solvable using regex, I'm not sure it's the best tool for the job. It's a very generic tool, where a non-genric solution is enough on the one hand, and on the other - it's not optimized for this task. Just to compare - the no-custom-classname ESLint rule recognizes invalid classes inside this large cva call pretty much immediately, and vice-versa. I think they're utilizing regex, but not relying solely on it (granted, I haven't looked very closely on their implementation, so might be wrong here).

PatrykWalach commented 8 months ago

The regexes above were really slow for me. And support only one pair of brackets. In this stack overflow post(%5D)%5C)%5B%5E)(%5D)%5C)%5B%5E)(%5D)%5C)) I've found a really fast version which supports three pairs, and can be expanded further

"tailwindCSS.experimental.classRegex": [
      ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "\"(.*?)\""],
]
Manas-Pratim-Dutta commented 7 months ago

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module

the above error is showing whenever I install Tailwind CSS IntelliSense and auto complete doesn't work

thecrypticace commented 3 months ago

This is a limitation of regexes not supporting recursion and needing to match on [^)]. I have some ideas on how to improve this in the future but, at least for now, I would use the regex that @PatrykWalach provided with a small tweak ([^\"]*) instead of (.*?).

Here's an example that supports single quotes, double quotes, and template literals.

{
  "tailwindCSS.experimental.classRegex": [
    ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "'([^']*)'"],
    ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "\"([^\"]*)\""],
    ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "`([^`]*)`"],
  ]
}

Now it doesn't support nested template literals for the same reason (no recursion in regex) but it otherwise works quite well.

bengry commented 3 months ago

Thanks @thecrypticace. If I want to have intellisense in multiple functions (e.g. cx, cva, clsx) - I need to repeat these 3 patterns for each, right? e.g.

{
  "tailwindCSS.experimental.classRegex": [
    // cva(...)
    ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "'([^']*)'"],
    ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "\"([^\"]*)\""],
    ["cva\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "`([^`]*)`"],
    // cx(...)
    ["cx\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "'([^']*)'"],
    ["cx\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "\"([^\"]*)\""],
    ["cx\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "`([^`]*)`"],
    // clsx(...)
    ["clsx\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "'([^']*)'"],
    ["clsx\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "\"([^\"]*)\""],
    ["clsx\\(([^)(]*(?:\\([^)(]*(?:\\([^)(]*(?:\\([^)(]*\\)[^)(]*)*\\)[^)(]*)*\\)[^)(]*)*)\\)", "`([^`]*)`"],
  ]
}
thecrypticace commented 3 months ago

That is correct

actually you could probably use a non-capturing group and one pattern. So replace cva with this (?:clsx|cva|cx)

bengry commented 3 months ago

actually you could probably use a non-capturing group and one pattern. So replace cva with this (?:clsx|cva|cx)

That's a great idea. Thanks!

It looks good enough for now, even if not perfect. I hope that a more robust (i.e. non-regex-based) solution will eventually be implemented. It'll also probably help with performance.

Thanks again!