Skip to content

Conversation

linspw
Copy link

@linspw linspw commented Aug 28, 2023

Hello everything is fine?
This Pull request has 3 objectives:

  • Help to better organize the vite plugin (separate responsibilities from roles)
  • Fix the bug where nuxt couldn't find the style files (because of the relative path) and it crashed.
  • Fix the lib virtual prefix, to use nuxt main pattern.

BUGS:

  1. The relative path of the target was making the nuxt load break, for some reason the path was generating a url that didn't match the file.

  2. The other sourcemap bug was caused by the \0 prefix, where the virtual is what nuxt is prepared to read (I imagine this change won't break vite builds).

#311
#290

Thanks in advance for your attention, feel free to adjust.

@@ -158,12 +176,16 @@ export function stylesPlugin (options: Options): Plugin {
)
) {
if (options.styles === 'none') {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early return

@@ -15,101 +15,122 @@ function isSubdir (root: string, test: string) {
return relative && !relative.startsWith('..') && !path.isAbsolute(relative)
}

// FOR SOME REASON: /�plugin-vuetify:/home/jesse/Documents/personal/nuxt/vuetify/lib/styles/main.sass?direct 16:53:03
const PLUGIN_VIRTUAL_PREFIX = "virtual:"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Islati
Copy link

Islati commented Sep 11, 2023

👍 Hope this helps

@linspw
Copy link
Author

linspw commented Sep 20, 2023

Hi @KaelWD .

If it's not a problem, could you see if this PR makes sense please?

@jd-solanki
Copy link

@KaelWD Can you please review it?

@mostafaznv
Copy link

any news?

@hi-reeve
Copy link

hi-reeve commented Oct 9, 2023

will this merged?

@borghol
Copy link

borghol commented Nov 26, 2023

Hello guys,

Any expected timeline on when this will be fixed?

It's the only thing that stops Nuxt users from being able to customize SASS variables.

@muhammadmahmoud98
Copy link

Hello, is there any news on this? hope it gets merged soon

@KaelWD
Copy link
Member

KaelWD commented Dec 8, 2023

This wasn't merged because it basically rewrites the entire plugin for no reason, if it's still relevant please only include changes necessary to fix the issue.

@franz-bendezu
Copy link
Contributor

Hi @KaelWD I wanted to inform you that I've created a new pull request (#328) addressing the feedback provided on the original pull request (#313).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants