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

fix types: accept a readonly array for configuration options #12818

Closed
wants to merge 2 commits into from

Conversation

pcorpet
Copy link
Contributor

@pcorpet pcorpet commented Mar 5, 2021

The webpack main function can get multiple configuration as an array. However this array doesn't need to be mutable. This fix makes it accept readonly arrays.

What kind of change does this PR introduce?

bugfix for typescript types declaration

Did you add tests for your changes?

No: I couldn't find typing tests.

Does this PR introduce a breaking change?

No: all types that were accepted before are still accepted, it accepts new types.

What needs to be documented once your changes are merged?

Nothing, this is a fix.


This change is Reviewable

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@sokra
Copy link
Member

sokra commented Mar 5, 2021

This is a generated file. Look into lib/webpack.js or lib/index.js and fix it in the jsdocs comments

@pcorpet
Copy link
Contributor Author

pcorpet commented Mar 5, 2021

This is a generated file. Look into lib/webpack.js or lib/index.js and fix it in the jsdocs comments

Thanks @sokra , I hadn't noticed.

I'm now stuck on microsoft/TypeScript#42768

			if (Array.isArray(options)) {
				/** @type {MultiCompiler} */
				compiler = createMultiCompiler(options, options);
				watch = options.some(options => options.watch);
				watchOptions = options.map(options => options.watchOptions || {});
			} else {
				/** @type {Compiler} */
				compiler = createCompiler(options);
				watch = options.watch;
				watchOptions = options.watchOptions || {};
			}

The isArray only guards propery against mutable arrays, not readonly arrays (see the bug in Typescript for deeper details).
Maybe we could cast the type of options in the else branch?

@pcorpet pcorpet force-pushed the pascal-readonly-array branch 2 times, most recently from eb78133 to 6438df8 Compare March 8, 2021 08:00
@pcorpet
Copy link
Contributor Author

pcorpet commented Mar 18, 2021

Can someone help me with the failing test please?

lib/webpack.js Outdated Show resolved Hide resolved
Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
@webpack-bot
Copy link
Contributor

@pcorpet Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@webpack-bot
Copy link
Contributor

@pcorpet The most important CI builds failed. This way your PR can't be merged.

Please take a look at the CI results from azure (1 errors / 0 warnings) and appveyor (success) and fix these issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants