remove @types/node dependency

[tjenkinson]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no (open to suggestions on how to test this)

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

• Installing Rollup as a devDependency hides typescript errors #3224

Description

Remove @types/node dependency because this is causing the node types to be included by default for any consumer of rollup.

Note "@types/node" is still brought in for development as it's a dev dependency of "@types/fs-extra", "@types/resolve", "@types/rimraf" and "rollup"(!)

fixes #3224

Here is an updated version of the repro which now correctly fails on an npm run build

[codecov]

Codecov Report

Merging #3226 into master will decrease coverage by 0.67%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3226      +/-   ##
==========================================
- Coverage   91.31%   90.64%   -0.68%     
==========================================
  Files         170      167       -3     
  Lines        5930     5911      -19     
  Branches     1797     1793       -4     
==========================================
- Hits         5415     5358      -57     
- Misses        310      336      +26     
- Partials      205      217      +12

Impacted Files	Coverage Δ
cli/run/batchWarnings.ts	25.61% <0%> (-3.42%)	⬇️
src/rollup/index.ts	93.93% <0%> (-3.1%)	⬇️
src/Chunk.ts	92.89% <0%> (-1.08%)	⬇️
src/ast/nodes/AssignmentExpression.ts	94.44% <0%> (-0.8%)	⬇️
src/Graph.ts	92.94% <0%> (ø)	⬆️
src/utils/mergeOptions.ts	90.47% <0%> (ø)	⬆️
src/Module.ts	97.3% <0%> (ø)	⬆️
src/utils/assignChunkIds.ts	100% <0%> (ø)	⬆️
src/ast/nodes/SwitchStatement.ts	100% <0%> (ø)	⬆️
src/utils/addons.ts	100% <0%> (ø)	⬆️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f4f012...8af7611. Read the comment docs.

[tjenkinson]

Also wondering if "@types/estree" should be a dev dependency?

[lukastaegert]

How do you intend to fix Rollup’s types for users of Rollup that typically do not install its devDependencies? Tell everyone what to install manually? Use a peerDependency and cause an warning for everyone, even though they do not care about the Node types?

[lukastaegert]

I am saying this because this has been discussed by countless people for a long time and just removing the dependency is just not a solution. If you can rework Rollup’s types so that we have our own version of Buffer that is fully compatible with Node's Buffer, now that would be a step forward. We could get rid of ESTree by using the less specific acorn types, but so far, nobody complained about ESTree. So any solution forward means that Rollup’s types need to be valid when you delete all devDependencies. Anything else will not be merged.

[tjenkinson]

Hmm I see.

nobody complained about ESTree

This just exports interfaces and doesn't register anything on any globals, so doesn't seem like an issue.

"@types/node" registers types on globals though. I guess the root problem here is the way typescript automatically brings in all types in the "@types" dir.

Maybe the advice for consumers should be to explicitly list the types you want with the types property and disable the automatic inclusion.

[shellscape]

@lukastaegert what's your take on the last comment? It seems reasonable but I'm no TS expert.

[lukastaegert]

This sounds like a reasonable documentation request

[tjenkinson]

ok. here's a suggested change to the docs: #3300