Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include type declarations in package #106

Closed
wants to merge 1 commit into from
Closed

Include type declarations in package #106

wants to merge 1 commit into from

Conversation

apancutt
Copy link

@apancutt apancutt commented May 6, 2021

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
Copy link
Owner

liady commented May 6, 2021

@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
Copy link
Author

apancutt commented May 6, 2021

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
Copy link

vis97c commented Jul 5, 2021

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

@VitalyLipko
Copy link

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
Copy link
Owner

liady commented Jul 24, 2021

@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
Copy link

@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
Copy link

Do we have updates? I thought the decision was made #106 (comment)

@robahtou
Copy link

robahtou commented Dec 1, 2021

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

@apancutt apancutt closed this by deleting the head repository Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsupported type in webpack 5
5 participants