nksaraf / vinxi

The Full Stack JavaScript SDK
https://vinxi.vercel.app
MIT License
1.91k stars 77 forks source link

feat: add comment directive to ignore style collection #324

Closed XiNiHa closed 2 months ago

XiNiHa commented 3 months ago

Continued from #316, cc @katywings

This PR adds // @vinxi-ignore-style-collection directive to ignore a module from the style collection. The directive only works when added at the top of the file, before having any meaningful code beside comments and whitespaces.

codesandbox[bot] commented 3 months ago

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders
Open Preview

changeset-bot[bot] commented 3 months ago

⚠️ No Changeset found

Latest commit: 1d85e5d850aac412f4520ceb6eb19bc73324126d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel[bot] commented 3 months ago

@XiNiHa is attempting to deploy a commit to the Nikhil Saraf's projects Team on Vercel.

A member of the Team first needs to authorize it.

katywings commented 2 months ago

@XiNiHa Very nice 🥳, thank you for your work! One question from my side: did you check how much this regex overall degrades the performance of the css collection and if it somehow can be skipped on repeated redundant collections (especially in prod)

XiNiHa commented 2 months ago

Not really :sweat_smile: I haven't had any concerns around performance since RegExp is mostly fast in JS and I didn't use any performance-critical features like lookahead. Also since it's only a build-time work so just adding a RegExp check wouldn't be that critical to performance (tbh replacing Babel with SWC would make much more impact here)

ryansolid commented 2 months ago

I mean if we decided to do this I don't see a way of avoiding the reading through the code like this. So we are going to give it a shot. If it slows things down considerably we would need to adjust it minorly or I suppose come up with a different solution. But I think we are sort of priced into something like this with the agreed on solution.