RFC: Archive (or refine the purpose of) this repository
sam3d opened this issue · 10 comments
Firstly, I'd like to give a massive thanks to @manuelodelain for taking over and maintaining this project for the last 18 months (especially as I haven't personally used Nuxt at all in that time, so my understanding and experience of the ecosystem right now is limited at best).
I've been thinking recently about whether this module does too much.
The main reason I made it, and the primary value that I've gotten out of it, is a maintained and updated list of existing rules so that they can be removed and overridden:
Lines 3 to 10 in 2e8f33b
This allows for flexibility and customisation of the SVG rules. So what did I do with all that newfound flexibility? Created a module that hardcoded more fixed rules to replace the existing ones! 🤦♀️ With the benefit of hindsight, this feels over-engineered and frustrating. Please feel free to disagree with me here, but defining your own rule as you need it in your own config is an extremely trivial task:
Lines 89 to 95 in 2e8f33b
As I don't know whether or not this module is still needed, I'd like to propose that we do one of the following things:
- Archive the repository (if it's no longer offering value in the current ecosystem)
- Publish a new major version that simply acts as an SVG rule "cleaner", and update the README.md to show how to use it to add your own SVG rules that can use whatever loaders you need/want
- Keep on maintaining this project as is with no change in direction
I shall await your feedback :)
I think the main advantages of this module (and the reason why I use it) are:
- provide an out of the box SVG integration, without the need to extend Webpack config. I agree it is not very complicated to add your own rules, but it is even easier here :)
- switch super easily between different methods of SVG integration (inline, sprite, external image...)
Nuxt 3 is here and the configuration becomes a bit more complicated.
Now the modules should work with Webpack or Vite.
I started to think about it recently and started a brand new version of the module.
You can see it on the next branch https://github.com/nuxt-community/svg-module/tree/next
The new module is agnostic and works with Webpack and Vite, thanks to Nuxt Kit.
I think it would add another advantage to this module.
I think such module has a purpose.
Sure you can say that this can be all done by hand but then the instructions on how to do it would still have to be available somewhere and that would likely be much less discoverable.
Also there are extra dependencies needed to enable this functionality so a module simplifies this a lot. Especially if you have to apply same configuration in many projects.
Thank you for both of your feedback so far! 😊
@rchl To be clear, the additional instructions would be in the README alongside the current install and setup instructions. I don't personally believe that makes it "much less discoverable". It would just be part of the correct setup of the module alongside the other (already needed) installation steps. It would be a bit more work but I don't think discoverability is an issue.
An interesting issue relevant to your point about "extra dependencies" - as a result of taking over the dependency management for the loaders, you are bound by the dependency versions it uses (instead of being able to self sufficiently manage your own loader dependencies). It's a tradeoff, like any other.
For completeness, here is the documentation required to use one of the loaders vs the documentation in a hypothetical self-setup scenario (which again, would likely have to be checked / copy pasted in either case anyway):
Current
npm install --save-dev @nuxtjs/svg// nuxt.config.js
export default {
buildModules: ["@nuxtjs/svg"],
svg: {
vueSvgLoader: {
// vue-svg-loader options
},
},
};<template>
<NuxtLogo />
</template>
<script>
import NuxtLogo from "~/assets/nuxt.svg?inline";
export default {
components: { NuxtLogo },
};
</script>Proposed
npm install --save-dev @nuxtjs/svg vue-svg-loader// nuxt.config.js
export default {
buildModules: ["@nuxtjs/svg"],
build: {
extend(config) {
config.module.rules.push({
test: /\.svg$/i,
oneOf: [{ resourceQuery: /inline/, use: "vue-svg-loader" }],
});
},
},
};<template>
<NuxtLogo />
</template>
<script>
import NuxtLogo from "~/assets/nuxt.svg?inline";
export default {
components: { NuxtLogo },
};
</script>Without using this module at all (using a built-in regex match)
npm install --save-dev vue-svg-loader// nuxt.config.js
export default {
build: {
extend(config) {
const svgRule = config.module.rules.find((rule) => rule.test.test(".svg"));
svgRule.test = /\.(png|jpe?g|gif|webp)$/;
config.module.rules.push({
test: /\.svg$/i,
oneOf: [{ resourceQuery: /inline/, use: "vue-svg-loader" }],
});
},
},
};<template>
<NuxtLogo />
</template>
<script>
import NuxtLogo from "~/assets/nuxt.svg?inline";
export default {
components: { NuxtLogo },
};
</script>Don't get me wrong, it definitely simplifies it a bit, but I wouldn't go so far as to say it "simplifies a lot". And not using this module lends a lot of additional flexibility to the end-user. So much so that I do genuinely believe it's worth considering whether this module in its current form is actually providing significant utility. Though again, I'm very open to a conversation on what that looks like :)
I guess my overarching policy is "configuration over convention" in situations where you only need to do something once. A Nuxt.js project may be fairly long lived, and the config is only interacted with at the beginning and when you need to make changes. It's not an ongoing burden, and perhaps my position can best be summarised by "don't have magic where you don't need magic".
Fwiw I think I'd also be happy to go down a path where we keep everything the same, but document explicitly in the README how to manually do in the config what this module does for you (like I did in the examples above). Both to reduce the ✨ magic ✨, and also encourage people not to install a dependency that they may not need.
@rchl To be clear, the additional instructions would be in the README alongside the current install and setup instructions.
OK. Sounds good. I thought that the module would not exist anymore in which case it would likely has less visibility.
An interesting issue relevant to your point about "extra dependencies" - as a result of taking over the dependency management for the loaders, you are bound by the dependency versions it uses
It's typically a positive thing for the user though when module maintainers take care of that since it frees the user of having to figure out which version to use (likely the latest versions of loaders are for webpack 5 and wouldn't work with Nuxt 2). And as long as dependencies are not pinned, it's easy to use the latest versions. I can only think of this becoming an issue if module maintainers don't care about the module anymore and it starts lagging with outdated major dependency versions.
Fwiw I think I'd also be happy to go down a path where we keep everything the same, but document explicitly in the README how to manually do in the config what this module does for you
Sounds perfect to me.
Here is a typical use case on my side.
npm install --save-dev @nuxtjs/svg
// nuxt.config.js
export default {
buildModules: ["@nuxtjs/svg"],
svgSpriteLoader: {
// ... config
},
};
// Component A
<template>
<NuxtLogo />
</template>
<script>
import NuxtLogo from "~/assets/nuxt.svg?inline";
export default {
components: { NuxtLogo },
};
</script>
// Component B
<template>
<NuxtLogo />
</template>
<script>
import NuxtLogo from "~/assets/nuxt.svg?sprite";
export default {
components: { NuxtLogo },
};
</script>
With very few lines of config I can switch from sprite to inline SVG.
@manuelodelain Sorry just to clarify, are you arguing for this module specifically? Or just for the use of resource queries
It was about all your scenarios and the fact that the current module doesn't simplify a lot the SVGs integration.
IMHO the possible bias here is that the scenarios compare the configuration with only one manner to integrate SVGs (inline here).
Usually I think a developer has to integrate SVGs in different ways in a project and I see this module as a response to that.
Would it be possible to release a patch version of the module where we only bump minor and patch dependencies to address some of their vulnerabilities?
Here's the new ranges I propose but haven't tested:
{
"dependencies": {
"file-loader": "^6.2.0",
"raw-loader": "^4.0.2",
"svg-sprite-loader": "^5.2.1",
"url-loader": "^4.1.1",
"vue-svg-loader": "^0.16.0"
},
"devDependencies": {
"nuxt": "^2.17.2",
"prettier": "^3.1.1"
}
}Here's the result of npm audit after applying these changes:
- 55 vulnerabilities (12 moderate, 33 high, 10 critical)
+ 32 vulnerabilities (10 moderate, 21 high, 1 critical)Running npm audit fix gets us a better result but this sometimes messes things up for me so I'm not sure how safe it is to publish these results without tests:
- 32 vulnerabilities (10 moderate, 21 high, 1 critical)
+ 19 vulnerabilities (8 moderate, 11 high)