-
-
Notifications
You must be signed in to change notification settings - Fork 331
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
Feat: Adding our CSS classes to Tailwind's intellisense #673
Feat: Adding our CSS classes to Tailwind's intellisense #673
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@niktek Given how much I'm juggling this week I'm going to set you as a reviewer on this one. I'd still like to make the final call and review before merge, but you may be able to jump in sooner than myself. See the primary conversation in the ticket to get caught up on this. |
@AdrianGonz97 this PR has been sitting idle for a couple weeks now. I'm not sure if you have any interest in returning to research CSS-in-JS transpiling. If not we might want to opt to close this out for now and return later. However, please see my proposal here. It's not a silver bullet, and only get us part of the way there, but may be something we should look into sooner than later: |
@endigo9740 I'm willing to give another deep dive into this again. I think I have fully recovered since my last CSS-in-JS venture 😂. Your suggestion in #650 (comment) seems like it should work! I'll give it a shot here. |
@AdrianGonz97 I know you're still working on this and testing things. But a quick review - from my perspective it looks like you're building a whole tool inside the Skeleton project to handle the transpiling process. I see quite a few additional commands, dependencies, scripts, etc. Something to consider here is is perhaps this would be better to split off to it's own package that Skeleton can then implement? It can be something you can own completely if you so choose, or we can fold it under the umbrella of Skeleton. I have no strong feelings either way. But consider for a moment that if we're struggling with this issue, how many others might as well. It might be great if this is something that could be opened up and provided to others in a reusable fashion. Let me know your thoughts! |
@endigo9740 Most of the scripts I've added were part of my experimental phase, so I'll be deleting them once I get more dialed in on what's actually necessary. Same goes for the added packages. I'm not too sure that this would work as a separate package, but I can look into that later. |
@endigo9740 This PR is almost complete. One problem that I've been dealing with is the additional bundle size that this plugin adds on top of our already considerably large bundle. It was adding about ~30kB to the bundle by itself, which was unacceptable. I figured out nice way to negate that completely by only allowing the plugin to function during development. Now we can have that sweet intellisense without the penalty of increasing our bundle size! One last thing to do is to add your suggestion #650 (comment). Might be a bit difficult to do because I'll have to convert to CSS-in-JS. We'll see how it goes! |
@endigo9740 |
Before submitting the PR:
npm run test
?branch -m new-branch-name
What does your PR address?
Progress tracking and experimenting for #650.
Running
npm install
to add the new dependenciesnpm run build:jss
to transpile theall.css
stylesheet