Fix sass types for TS 4.7+ Node16/NodeNext module resolution

[Goodwine]

Fixes #1714

[nex3]

devDependencies doesn't make sense—those aren't used by packages that depend on this package, and this package.json isn't ever used for local development. I think this should just be normal dependencies.

[ntkme]

@nex3 @Goodwine I don't think this should be added at as dependencies, because it is the user's choice to use TS or JS. When using pure JS, we shouldn't need any types to be installed as a dependency. If this is affecting the TS compilation of this project, it should indeed be devDependencies.

[Goodwine]

Wouldn't the @types/node package be downloaded regardless of whether it being declared as dependencies vs devDependencies? or do devDependencies get skipped if sass gets declared as non-dev dependency?

[nex3]

do devDependencies get skipped if sass gets declared as non-dev dependency?

This is correct, unless I'm severely misunderstanding. Sometimes it's necessary to include a dependency that's not used by some users because it's necessary for others.

[devversion]

FWIW: Node packages (even when shipping .d.ts) usually never add a dependency to just satisfy TypeScript type-checking for users. Instead:

• Users either have to set up @types/node themselves, or the library authors place a directive implying the necessary type like: /// <reference types="node" /> into their .d.ts
• Or, users most of the time just disable type-checking of external libraries. e..g by setting skipLibCheck: true.

Also just to make this clear: The devDependencies will never be installed for users, only for the authors of the package when e.g. yarn/npm is executed inside the folder containing the package.json.

TL;DR: Adding @types/node as a dependency is pretty uncommon. It should be removed. It's also not a big deal though and doesn't impact anyone negatively.

[nex3]

If including @types/node doesn't impact anyone negatively, and means that users can freely use the TS types without needing to manually install additional dependencies, why not include it?

[ntkme]

Well, this may become an addition to the reasons we get bloated node_modules.

Also, having @types/node lock to version 18 as part of dependencies is not really a good idea given that end users might want to install other versions (e.g. 16) if that is the version of node they are using. If it is devDependencies, then it does not matter as devDependencies are not installed for end users.

[devversion]

👍 @ntkme

yep, mainly for good practice of not increasing the dependency tree for people not using the TS types, which is very likely the majority here. This is peanuts in size, but a general question of best practice.

You also don't have a peer dependency/ or dependency on TypeScript itself, making guarantees with what version your types work

[nex3]

Also, having @types/node lock to version 18 as part of dependencies is not really a good idea given that end users might want to install other versions (e.g. 16) if that is the version of node they are using.

This is a good point—we should look into making the version range here as wide as possible while still including the APIs we use (I think only Buffer?).

You also don't have a peer dependency/ or dependency on TypeScript itself, making guarantees with what version your types work

Ideally, we'd be able to specify a dependency of the form "if the downstream user has TypeScript installed, then it must have this version", but unfortunately peer dependencies will produce warnings even if the downstream user doesn't have TypeScript.

[devversion]

Thanks for fixing this! I just added a little comment above (which you may want to consider, or not)