liady / webpack-node-externals

Easily exclude node modules in Webpack
MIT License
1.3k stars 62 forks source link

Include type declarations in package #106

Closed apancutt closed 1 year ago

apancutt commented 3 years ago

Fixes #105

Adds JSDoc to allow declaration files to be generated by tsc. TypeScript users would no longer be required to install @types/webpack-node-externals.

liady commented 3 years ago

@apancutt thank you so much for this! Highly appreciated. Do we also need to add JSDoc to the functions in utils.js? Also - can it break anything for existing users? Is it fully compatible with @types/webpack-node-externals?

apancutt commented 3 years ago

No need for JSDoc in utils.js right now since those functions aren't typically consumed - @types/webpack-node-externals also does not include typings for them so this won't introduce any breaking change.

Since the return type is declared loosely as (...args: any) => any it will be compatible with all versions of Webpack where config.externals can be a function (all?). This differs to @types/webpack-node-externals which returns a more accurate type of Webpack.ExternalsFunctionElement but 1) that is what causes the problem between v4 and v5, and 2) I'm not sure it's possible - even if we wanted to - return a type exposed by Webpack without adding a specific version of Webpack as a dependency of webpack-node-externals which will cause problems for users.

If users have already installed @types/webpack-node-externals then the typings provided by this package should take precedence. However, if users do experience a conflict, simply removing @types/webpack-node-externals from their dependencies will resolve it (they should do this either way).

While this is not a breaking change, it possibly should be a minor version bump to indicate to consumers that something significant has changed - highlighting that they should remove @types/webpack-node-externals if present.

vis97c commented 3 years ago

Any further news on this? it would be a huge improvement for this package to not depend on some ambigous typings.

VitalyLipko commented 3 years ago

I came across with problem when client part of my project depends on webpack 5 typings, but server part use previous versions typings included in @types/webpack-node-externals. So why that MR does not merge? I think not only i have issues with outdated typings.

liady commented 3 years ago

@VitalyLipko I'm mainly concerned by it breaking for existing users who are using @types/webpack-node-externals. Thinking about making this a breaking change, just for this reason. What do you think?

VitalyLipko commented 3 years ago

@VitalyLipko I'm mainly concerned by it breaking for existing users who are using @types/webpack-node-externals. Thinking about making this a breaking change, just for this reason. What do you think?

I think release version with breaking changes is normal practice. Sooner or later it must be happens, so users have to keep themselves in the loop with change in dependencies and update to actual versions.

VitalyLipko commented 3 years ago

Do we have updates? I thought the decision was made https://github.com/liady/webpack-node-externals/pull/106#issuecomment-886105021

robahtou commented 2 years ago

Just wanted to drop a comment to bump this into view again. Any updates?