cmalven / vite-plugin-sass-glob-import

Use glob syntax for imports in your main Sass or SCSS file.
MIT License
18 stars 7 forks source link

vite@3 support #5

Closed gnbaron closed 2 years ago

gnbaron commented 2 years ago

First, thanks for setting this up.

I'm trying to use this plugin with vite@3 and apparently, the return type is not compatible with PluginOption.

Type 'Plugin' is not assignable to type 'PluginOption'.
  Type 'Plugin' is not assignable to type 'Plugin_2'.
    Types of property 'apply' are incompatible.
    ...

Do you have plans to upgrade it?

cmalven commented 2 years ago

@gnbaron Thank you for sharing this. I've been using this plugin with Vite 3 since it was released and haven't seen this issue. Do you have a repo you can share where this is happening?

gnbaron commented 2 years ago

Really weird you not see this @cmalven, package.json says Vite peer dependency version is ^2.7.6, which means the Plugin you are exporting is a v2 plugin, while my Vite 3 project is expecting a v3 plugin definition. I'm not quite sure if both are compatible, based on the error it doesn't look like they are.

Here is a quick repro repository, straight out of Vite react+ts template, I just added vite-plugin-sass-glob-import plugin.

Screen Shot 2022-10-31 at 16 10 25

cmalven commented 2 years ago

@gnbaron Thanks for sharing. You're correct that the peerDependencies should be bumped - I neglected to do so because, like I said, I've been using this with 3.x versions of Vite for a long time and never had an issue. I'm actually still not exactly sure why your reproduction repo is throwing this error and none of my other 3.x projects are.

I'll look into it. This plugin doesn't get a ton of downloads, but it definitely gets enough that I'm surprised this issue wouldn't have popped up for someone before now.

gnbaron commented 2 years ago

No worries @cmalven! Let me know if you need help with that.

jrochkind commented 2 years ago

I just started trying this out too, seems to work fine with vite 3 indeed. Thanks for the useful plugin!

But I also noticed the dependency issue.

Note that vite is actually listed as a straight dependency, not just a peerDependency! It's listed twice (with slightly different minimum versions!), which I don't think is probably desirable.

https://github.com/cmalven/vite-plugin-sass-glob-import/blob/2cad5760491f7e341c12b1575f8c96881e4754ac/package.json#L29-L33

In addition to changing the peerDependency (either bump or just expand the range to include both 2 and 3?), can you drop it from dependencies so it's only peerDependencies?

I am in the middle of transitioning a project from webpack to vite, and just ran into this -- what a coincidence only a week after @gnbaron filed this issue?

I'm not sure if it's actually causing me any problems or not, I'm not personally having the probelm @gnbaron reports... but it is really bloating my node_modules (and my yarn.lock) with a whole additional copy of vite 2 and all it's dependencies, since yarn installs em all since it's listed as a dependency! I would prefer to keep things tidy.

Would a PR help?

cmalven commented 2 years ago

Thanks @jrochkind for adding to this conversation.

I just released 1.4.0 (https://github.com/cmalven/vite-plugin-sass-glob-import/releases/tag/1.4.0) that changes things in two ways:

As far as I can tell, no other updates are needed for the plugin to work with Vite v3, and the issue @gnbaron is having seems to be related to using "type": "module" in package.json which I'll need to investigate separately.

Would you mind installing 1.4.0 locally and let me know if everything works out for you?

gnbaron commented 2 years ago

Thanks for working on this @cmalven! I've just upgraded it and the TypeScript error is gone. 🎉

I'm also migrating from Webpack @jrochkind haha. Not sure if I didn't make it clear, but I was only having that TypeScript error, besides that, everything seems to be working as expected.

jrochkind commented 2 years ago

Thank you! I will not have time to test this until next week, but i will get back to you next week!

cmalven commented 2 years ago

@gnbaron Re-reading your original message it is clear to me that you were only referring to the typescript error. I get a different error - one that actually breaks your build - running your sample repo locally so I just got things mixed up and assumed that was the error. Glad things are working now!

jrochkind commented 2 years ago

1.4.0 seems to be working well for me, thanks @cmalven !