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

Type issues in webpack-merge #141

Closed
3 tasks done
sdc224 opened this issue Jul 13, 2020 · 35 comments · Fixed by #155
Closed
3 tasks done

Type issues in webpack-merge #141

sdc224 opened this issue Jul 13, 2020 · 35 comments · Fixed by #155

Comments

@sdc224
Copy link

sdc224 commented Jul 13, 2020

Unable to use devServer property inside webpack.config.ts, when writing webpack configuration in TS file.

  • Using updated webpack-merge
  • Using NodeJS v10.16
  • Using Windows 10

While I'm in webpack-merge v4 with @types/webpack-merge, it was working fine. Now after updating webpack-merge to 5, I have removed the @types/webpack-merge(as per CHANGELOG). After that it is throwing TS error

Object literal may only specify known properties, and 'devServer' does not exist in type 'Configuration'.ts

webpack.config.dev.ts

import webpack from "webpack";
import { merge as webpackMerge } from "webpack-merge";
import baseConfig from "./webpack.common";
import paths from "../paths";

const config: webpack.Configuration = webpackMerge(baseConfig, {
	mode: "development",
	devtool: "#inline-source-map",
	devServer: {
		contentBase: paths.build,
		compress: true,
		open: true
	}
});

export default config;
@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

@sdc224 Can you try adding @types/webpack-dev-server to your project as it extends webpack's regular types with the devServer bit?

Likely @types/webpack-merge was pulling that in. I should at least document this use case as I'm not sure if we should pull the types automatically given you likely want to manage webpack-dev-server types separately from webpack-merge.

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

@types/webpack-dev-server It is already added.

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

@types/webpack-dev-server version is 3.11.0

@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

Ok, I'll check this better detail then. It might have to do with the way I wired Configuration together at the source and it's not picking up the dev server types for some reason.

I wonder if we should even go as far as to decouple Configuration from merge and treat it as a generic. You would then have to do something like <Configuration>webpackMerge(...).

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

Are you referring to this?

import { Configuration } from "webpack";
import { merge as webpackMerge } from "webpack-merge";
import baseConfig from "./webpack.common";
import paths from "../paths";
import WebpackDevServer from "webpack-dev-server";

const config = <Configuration & WebpackDevServer.Configuration>(
	webpackMerge(baseConfig, {
		mode: "development",
		devtool: "#inline-source-map",
		devServer: {
			port: 5000,
			contentBase: paths.build,
			compress: true,
			open: true
		}
	})
);

export default config;

@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

@sdc224 Exactly. I haven't implemented the idea yet but it's the first thing that came to my mind.

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

Yes, that's why it is still showing the issue.

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

Can you tell me how it was working in @types/webpack-merge? Maybe we can implement the same here.

@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

@sdc224 That's a good question. Let me research a bit.

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

@bebraw 😅. Are we going to get the fix in next release?

@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

Yup, I'll find some way. 👍

It looks like @types/webpack-merge is doing simply import { Configuration } from 'webpack';. It feels like a subtle difference somewhere.

@sdc224
Copy link
Author

sdc224 commented Jul 13, 2020

Ok thanks man, so quick responses 😄. Hoping to hear from you very soon.

@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

@sdc224 Can you set up a small repository that shows the type error? At least VS Code seems to find the types properly against a simple config.

@bebraw
Copy link
Member

bebraw commented Jul 13, 2020

@sdc224 Here's my TS setup (no type errors): https://github.com/bebraw/webpack-merge-ts-debug . I imagine there's some difference I missed.

@sdc224
Copy link
Author

sdc224 commented Jul 14, 2020

@bebraw Yes man, the catch is in the below line written in tsconfig.json

"include": ["src", "types"]

as your webpack config is outside of src, tsc doesn't find any error. However, if you proceed with VSCode, it will show you this error without any project config.

@sdc224
Copy link
Author

sdc224 commented Jul 14, 2020

@sdc224 Can you set up a small repository that shows the type error? At least VS Code seems to find the types properly against a simple config.

You can go through this repository

@bebraw
Copy link
Member

bebraw commented Jul 14, 2020

Thanks for the repository.

I learned that if I set

import { Configuration as WebpackConfiguration } from "webpack";
import { Configuration as WebpackDevServerConfiguration } from "webpack-dev-server";

interface Configuration extends WebpackConfiguration {
  devServer?: WebpackDevServerConfiguration;
}

within webpack-merge type definition, it works. The problem is that this couples the utility with webpack-dev-server as well. I found the solution from DefinitelyTyped/DefinitelyTyped#27570 .

I have to think more about this one. It's a little weird as @types/webpack-dev-server clearly injects its extension to webpack's Configuration object but it's not being picked up correctly for some reason. I think that's the core problem.

@bebraw
Copy link
Member

bebraw commented Jul 14, 2020

Here's another solution. Add this bit to the configuration:

import WebpackDevServer from "webpack-dev-server";

declare module "webpack" {
	interface Configuration {
		devServer?: WebpackDevServer.Configuration;
	}
}

The open question is why is this needed now?

@bebraw
Copy link
Member

bebraw commented Jul 14, 2020

@sdc224 I noticed your typeRoots field is in the wrong place. It should be behind compilerOptions. Moving it there doesn't change anything by the looks of it, though.

It's important to set that for ts-node to work correctly.

It feels like something else is missing as the dev server types aren't being applied with this change.

@bebraw
Copy link
Member

bebraw commented Jul 15, 2020

@sdc224 Would <Configuration>merge work for you? I think I could do the change without breaking backwards compatibility? Any better ideas?

I guess the other option would be to figure out why that webpack-dev-server configuration extension doesn't get applied in this particular case. It's tricky to say how to debug this, though.

@sdc224
Copy link
Author

sdc224 commented Jul 15, 2020

Sorry for so late reply, was occupied on some problems.

Here's another solution. Add this bit to the configuration:

import WebpackDevServer from "webpack-dev-server";

declare module "webpack" {
	interface Configuration {
		devServer?: WebpackDevServer.Configuration;
	}
}

The open question is why is this needed now?

This is how I am doing right now 🤣

@sdc224
Copy link
Author

sdc224 commented Jul 15, 2020

@sdc224 Would <Configuration>merge work for you? I think I could do the change without breaking backwards compatibility? Any better ideas?

I guess the other option would be to figure out why that webpack-dev-server configuration extension doesn't get applied in this particular case. It's tricky to say how to debug this, though.

@bebraw That merge is not working for me

@sdc224
Copy link
Author

sdc224 commented Jul 15, 2020

@sdc224 I noticed your typeRoots field is in the wrong place. It should be behind compilerOptions. Moving it there doesn't change anything by the looks of it, though.

It's important to set that for ts-node to work correctly.

Surely I will look into this, I totally missed it

@bebraw
Copy link
Member

bebraw commented Jul 16, 2020

I started a branch with the generic. See https://github.com/survivejs/webpack-merge/blob/feat/ts-generic/test/merge.test.ts#L16 .

That said, it looks like webpack-dev-server is patching the type correctly even within this repository.

I wonder if we're looking at a TypeScript configuration issue here?

@trulysinclair
Copy link

Hey peeps, expanding on the type issues, the types allow

import WebpackMerge from 'webpack-merge';

WebpackMerge.merge(...)

but returns (node:4415) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'merge' of undefined,
and trying to use as expected also return a TypeError.
Screenshot_14

@bebraw
Copy link
Member

bebraw commented Jul 17, 2020

@TheGrimSilence Can you change from

import WebpackMerge from 'webpack-merge';

to

import * as WebpackMerge from 'webpack-merge';

or

import { merge as WebpackMerge } from 'webpack-merge';

I had to change the exports to avoid const { default as merge } = require('webpack-merge') on CommonJS. That's why merge is a regular export now.

@sdc224
Copy link
Author

sdc224 commented Jul 18, 2020

I started a branch with the generic. See https://github.com/survivejs/webpack-merge/blob/feat/ts-generic/test/merge.test.ts#L16 .

That said, it looks like webpack-dev-server is patching the type correctly even within this repository.

I wonder if we're looking at a TypeScript configuration issue here?

What do you think? Should we raise in webpack-dev-server?

I wonder how it was working in @types/webpack-merge in webpack-merge v4

@sdc224
Copy link
Author

sdc224 commented Jul 31, 2020

Guys any update on this?

@bebraw
Copy link
Member

bebraw commented Jul 31, 2020

Hi,

I'm a bit stuck on this one as I'm not sure if the issue is on the typing side or something in TypeScript we don't understand.

Likely external type definitions are more powerful as they can model a bit more. I bet it's something "obvious" we are missing.

@sdc224
Copy link
Author

sdc224 commented Jul 31, 2020

One thing I need to mention, I don't know any explanation for that, but for me it is right now working fine😅😅, I mean, working without any type errors

@dzintars
Copy link

dzintars commented Aug 2, 2020

Similar issue there. Today just made a fresh yarn install for the existing project and got

TSError: ⨯ Unable to compile TypeScript:
webpack.dev.ts:8:39 - error TS2349: This expression is not callable.
  Type 'typeof import("/home/dzintars/code/front/.yarn/cache/webpack-merge-npm-5.0.9-c8179aa67c-7d31955ae5.zip/node_modules/webpack-merge/dist/index")' has no call signatures.

8 const config: webpack.Configuration = merge(common, {

    at createTSError (/home/dzintars/code/front/.yarn/$$virtual/ts-node-virtual-37c47ab5ad/0/cache/ts-node-npm-8.10.2-b4fe5a56b0-cd6e023e07.zip/node_modules/ts-node/src/index.ts:434:12)
    at reportTSError (/home/dzintars/code/front/.yarn/$$virtual/ts-node-virtual-37c47ab5ad/0/cache/ts-node-npm-8.10.2-b4fe5a56b0-cd6e023e07.zip/node_modules/ts-node/src/index.ts:438:19)
    at getOutput (/home/dzintars/code/front/.yarn/$$virtual/ts-node-virtual-37c47ab5ad/0/cache/ts-node-npm-8.10.2-b4fe5a56b0-cd6e023e07.zip/node_modules/ts-node/src/index.ts:578:36)
    at Object.compile (/home/dzintars/code/front/.yarn/$$virtual/ts-node-virtual-37c47ab5ad/0/cache/ts-node-npm-8.10.2-b4fe5a56b0-cd6e023e07.zip/node_modules/ts-node/src/index.ts:775:32)
    at Module.m._compile (/home/dzintars/code/front/.yarn/$$virtual/ts-node-virtual-37c47ab5ad/0/cache/ts-node-npm-8.10.2-b4fe5a56b0-cd6e023e07.zip/node_modules/ts-node/src/index.ts:858:43)
    at Module._extensions..js (internal/modules/cjs/loader.js:1277:10)
    at Object.require.extensions.<computed> [as .ts] (/home/dzintars/code/front/.yarn/$$virtual/ts-node-virtual-37c47ab5ad/0/cache/ts-node-npm-8.10.2-b4fe5a56b0-cd6e023e07.zip/node_modules/ts-node/src/index.ts:861:12)
    at Module.load (internal/modules/cjs/loader.js:1105:32)
    at Function.external_module_.Module._load (/home/dzintars/code/front/.pnp.js:17771:14)
    at Module.require (internal/modules/cjs/loader.js:1145:19)

I have Yarn PnP greenfield setup there:

» yarn why webpack-merge
└─ front@workspace:.
   └─ webpack-merge@npm:5.0.9 (via npm:latest)
» node --version
v14.7.0
» npm --version 
6.14.7
» yarn --version        
2.1.1
» tsc --version
Version 3.9.7

Not sure which update exactly causes this. Will leave this comment. Have installed all required @types, etc.

@bebraw
Copy link
Member

bebraw commented Aug 3, 2020

@dzintars Can you set up a small repository showcasing the error? Note that you should do import { merge } from 'webpack-merge' as I was forced to push the imports there.

I could expose merge as a default export, though, now that I think of it. That would show up as default in CommonJS but it would provide backwards-compatibility for TS so it sounds like a good idea.

@bebraw
Copy link
Member

bebraw commented Aug 3, 2020

@dzintars I added support for import merge from "webpack-merge" to 5.1.0 so now TS import is backwards-compatible.

@dzintars
Copy link

dzintars commented Aug 3, 2020

@bebraw Good morning. :) Thank you very much! Seems at least my issue is solved with 5.1.0.

@bebraw
Copy link
Member

bebraw commented Aug 3, 2020

@dzintars Ok, cool. I think this issue is something else as webpack-dev-server doesn't seem to patch webpack type correctly for this specific case and it's still unclear why.

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 a pull request may close this issue.

4 participants