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

Add support for TypeScript config path mapping in CSS files #1106

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Dec 5, 2024

This is a work in progress with a handful of things that need improvements regarding stylesheet loading / editing when in a v4 project.

Fixes #1103
Fixes #1100

  • Recover from missing stylesheet imports
  • Recover from unparsable stylesheet imports (not sure if possible)
  • Read path aliases from tsconfig.json
  • Log errors from analyzing CSS files during the resolve imports stage (the cause of v4: Intellisense attempts to load CSS config from wrong file #1100)
  • Watch for tsconfig.json file changes and reload when they change (or maybe only when the list of seen paths do)
  • Consider path aliases when doing project discovery
  • Consider path aliases when loading the design system
    • Allow in @import
    • Allow in @reference
    • Allow in @config
    • Allow in @plugin
  • Consider path aliases when producing diagnostics
    • Allow in @import
    • Allow in @reference
    • Allow in @config (nothing to do here)
    • Allow in @plugin (nothing to do here)
  • Consider path aliases when generating document links
    • Allow in @import (no upstream support; non-trivial)
    • Allow in @reference (no upstream support in @import; non-trivial)
    • Allow in @config
    • Allow in @plugin
  • Consider path aliases when offering completions
    • Allow in @import (no upstream support; non-trivial)
    • Allow in @reference (no upstream support in @import; non-trivial)
    • Allow in @config
    • Allow in @plugin

@thecrypticace thecrypticace force-pushed the fix/better-import-handling branch 2 times, most recently from a5b5ce8 to 99f66e2 Compare December 13, 2024 15:06
@thecrypticace thecrypticace force-pushed the fix/better-import-handling branch 4 times, most recently from a42a518 to 75ba3b6 Compare December 15, 2024 13:52
pvds added a commit to pvds/mikrouli that referenced this pull request Dec 26, 2024
@thecrypticace thecrypticace force-pushed the fix/better-import-handling branch 3 times, most recently from 1c6ec15 to 2934e37 Compare January 7, 2025 23:15
@thecrypticace thecrypticace changed the title Recover from missing imports when reloading the design system Add support for TypeScript config path mapping in CSS files Jan 8, 2025
@thecrypticace thecrypticace marked this pull request as ready for review January 9, 2025 16:50
@thecrypticace thecrypticace force-pushed the fix/better-import-handling branch 2 times, most recently from 64eb3e9 to 85b1042 Compare January 9, 2025 19:30
@pvds
Copy link

pvds commented Jan 11, 2025

@thecrypticace is there an insiders version to test the current changes?

@thecrypticace
Copy link
Contributor Author

@pvds Not yet no.

If you want you can test this by cloning / building the repo tho. It shouldn't be too difficult (you might need to tweak these steps a little if you're on Windows):

# Clone the repo
git clone https://github.com/tailwindlabs/tailwindcss-intellisense -b fix/better-import-handling
cd tailwindcss-intellisense

# Install and build
pnpm install
pnpm run -r build

# Open with vscode
code .

After doing the above you can show the debug sidebar in VScode and click launch client. That should launch an extension development host process that lets you test out the extension by opening a project.

@thecrypticace
Copy link
Contributor Author

I also still need to rebase this PR b/c of some fixes I made / released. Planning on doing that today or maybe Monday.

@pvds
Copy link

pvds commented Jan 11, 2025

UPDATE @thecrypticace does the following todo check mean autocomplete in templating is not supported yet in this this PR's branch:

  • Consider path aliases when offering completions
    • Allow in @import (no upstream support; non-trivial)

@thecrypticace I use Webstorm on a Mac and just tested it on a project I'm working on.

Unfortunately using custom @import rules does not seem to work yet

Webstorm using local build version of language server

I changed the version to 0.0.28-local-test-version

image

Verified whether the local build of the language service works (without custom imports)

image

Used CSS

image

Using custom imports

Uncommented the custom @import lines above > unfortunately does not work yet

image

@thecrypticace
Copy link
Contributor Author

thecrypticace commented Jan 11, 2025

@pvds Can you provide a link to your project / repo for me to test with? If you're testing web storm you'd have to point it at the custom language server build but I'm not sure how to do that right now.

@thecrypticace
Copy link
Contributor Author

… does the following todo check mean autocomplete in templating is not supported yet…

No, it means that when typing something like @import "$lib/<cursor here> that we'd not be able to suggest files. But the imports themselves should be handled by the language server just fine.

@pvds
Copy link

pvds commented Jan 11, 2025

UPDATE: I added some test css files and they work so it's not that @import breaks autocomplete by itself this is in the branch repro/import-autocomplete

It has something to do with these imports:

@import "$lib/styles/theme.css" layer(theme);
@import "$lib/styles/base.css" layer(base);
@import "$lib/styles/utilities.css" layer(utilities);

@pvds Can you provide a link to your project / repo for me to test with? If you're testing web storm you'd have to point it at the custom language server build but I'm not sure how to do that right now.


@thecrypticace this is the project: https://github.com/pvds/mikrouli/

Autocomplete works in the main branch but in feature/import-css it does not

Some notes

  • I use bun as a package manager but you could use pnpm, just note that the package.json scripts use bun.
  • readme should have all the info you need
  • see the src/app.css

@thecrypticace
Copy link
Contributor Author

Thanks for the repro! I can reproduce the issue but I'm not 100% certain why yet. I did verify that the CSS imports look to be handled correctly but it's still creating more "projects" than it should be so something's definitely going wrong somewhere. I'll see what I can figure out.

@pvds
Copy link

pvds commented Jan 11, 2025

Thanks for the repro! I can reproduce the issue but I'm not 100% certain why yet. I did verify that the CSS imports look to be handled correctly but it's still creating more "projects" than it should be so something's definitely going wrong somewhere. I'll see what I can figure out.

You're welcome, I will also try to spend some time figuring out when it exactly breaks

@pvds
Copy link

pvds commented Jan 11, 2025

@thecrypticace I think I found it:

Applying the utilities layer to an @utility breaks the intellisense autocomplete.

/* this breaks autocomplete
@layer utilities {
    @utility section {
        @apply px-4 py-2;
    }
}

/* this also breaks autocomplete*/
@import "$lib/styles/utilities.css" layer(utilities);

If you pull the latest changes in the main branch intellisense should work.

@thecrypticace
Copy link
Contributor Author

Ah yeah that will break one part of it b/c @utility cannot be nested. But there's still some other issue as well. I'll look more into this later this evening.

@pvds
Copy link

pvds commented Jan 11, 2025

Ah yeah that will break one part of it b/c @utility cannot be nested. But there's still some other issue as well. I'll look more into this later this evening.

It would make sense to me that in the release version of v4 applying a layer to @utility would trigger an error.
Silently failing and crippling intellisense is not great DX. Or it should behave graceful by not breaking intellisense and triggering a warning.

Curious to see what the other issue is.

@thecrypticace
Copy link
Contributor Author

Silently failing and crippling intellisense is not great DX. Or it should behave graceful by not breaking intellisense and triggering a warning.

Yeah, very much agreed. IntelliSense already does a bit of work to recover from otherwise invalid CSS for imports. I'll see if I can figure out a good solution for this as well.

@thecrypticace
Copy link
Contributor Author

Curious to see what the other issue is.

I figured it out and it's also ultimately caused by the @utility nesting. I need to tweak how projects are initialized to cleanup the messages around them to make the info more accurate.

@thecrypticace
Copy link
Contributor Author

I've opened an issue for myself to look into better error recovery mechanisms for IntelliSense as well but in your case making sure the @utility isn't nested should be all that's needed.

I plan on adding detection for things like this so we can warn.

@pvds
Copy link

pvds commented Jan 12, 2025

I've opened an issue for myself to look into better error recovery mechanisms for IntelliSense as well but in your case making sure the @utility isn't nested should be all that's needed.

I plan on adding detection for things like this so we can warn.

Sounds good!

@pvds
Copy link

pvds commented Jan 16, 2025

@thecrypticace are you planning to implement the non-trivial todos in this PR or just solve the conflicts?

@thecrypticace
Copy link
Contributor Author

Just the conflicts for now. I'll open a separate issue once this one is merged to look into the other stuff.

@thecrypticace thecrypticace force-pushed the fix/better-import-handling branch from 85b1042 to 4d9917c Compare January 17, 2025 23:54
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.

Custom @import breaks Intellisense on v4 beta v4: Intellisense attempts to load CSS config from wrong file
2 participants