search

swup / preload-plugin

A swup plugin for preloading pages to speed up navigation 🚀
https://swup.js.org/plugins/preload-plugin
MIT License
14 stars 14 forks source link

Move `@swup/plugin` from `dependencies` to `devDependencies` #59

Closed hirasso closed 1 year ago

hirasso commented 1 year ago

Should prevent auto-installation of node_modules inside package after npm i @swup/preload-plugin, if I'm not mistaken. If this works, should also be fixed in scroll-plugin.

hirasso commented 1 year ago

@gmrchk ok, apparently I'm still missing the full picture here. It'll probably easier if we discuss the problem and possible solution in person.

daun commented 1 year ago

It's definitely a normal dependency as we're including a method from that package.

@hirasso What's the issue with having a node_modules folder inside another package? That's how npm resolves different versions between packages. What we should do is not pin the plugin version but require a normal semver range.

hirasso commented 1 year ago

I should probably RTFM for package.json before creating PRs. Obviously I haven't yet fully understood NPMs dependencies concept, yet. Sorry for the noise 😅

daun commented 1 year ago

Haha, no worries!

hirasso commented 1 year ago

So, even after reading through this I don't understand why moving @swup/plugin to devDependencies would pose a problem. Looking forward to be enlightened by you guys in our next call 💡🙏

daun commented 1 year ago

Users of swup could have their production pipeline set up to not install dev dependencies via --production. In that case, none of the plugins that require the base class would have access to it since it won't be installed. It's not very common, but it's a possibility. Also, it's a direct dependency, not something only required while developing it like a linter or test runner.

hirasso commented 1 year ago

NOW I get it. Thanks @daun !