andywer / webpack-blocks

📦 Configure webpack using functional feature blocks.
MIT License
2.97k stars 94 forks source link

Establish compatibility of typescript block with webpack v4 - fixes #296 #297

Closed s-h-a-d-o-w closed 4 years ago

s-h-a-d-o-w commented 5 years ago

While preparing this, I of course took a closer look at what the configuration I've based this on (see: https://github.com/s-panferov/awesome-typescript-loader/issues/534#issuecomment-394368316 ) actually does and whether/why they are necessary - details see below. Obviously, just let me know if you want something changed. Like - upgrading TS to v3 may not be necessary but with this being part of a new major release anyway, I figured it would make sense. Oh yeah and I'm not familiar with your tests and what the snapshots should look like, so why merely updating the snapshots added thousands of LOC and whether that's good/bad/neutral... don't know.

  1. Moving TsConfigPathsPlugin to resolve gets rid of resolver.ensureHook is not a function But leads to Unexpected token possibly because of JSX
  2. Unexpected token is resolved by useBabel: true
  3. Since I guess the point of webpack-blocks is reasonable defaults, I've included these nonessential options:
s-h-a-d-o-w commented 5 years ago

Just saw that CI failed - help with that would be appreciated because I have no clue why PathPlugin expands to this huge object when TsConfigPathsPlugin is moved to resolve.

vlad-zhukov commented 5 years ago

@s-h-a-d-o-w There is a line in sample-app/__tests__/webpack-config.test.js that simplifies printing of plugins to just names. You'd need to add a similar line for resolve.plugins and rerun tests to generate a new snapshot. Also don't forget about the prod config too!

s-h-a-d-o-w commented 5 years ago

Hm... alright, I don't know what's going on here because you're already testing tsx with that sample-app and it passes. I didn't see that before I started working on this.

Why I get a TypeError in my own app and others using awesome-typescript-loader as well... who knows. But I guess this PR is simply not necessary.

jvanbruegge commented 5 years ago

You need to configure JSX in your tsconfig, do you have that?

s-h-a-d-o-w commented 5 years ago

Well... it was set "preserve" (I use next.js - guess they deal with JSX at some other point). Setting it to "react" did the trick for problem 2 and useBabel is no longer required.

Thanks for making me aware of this! But the major question (that TypeError) unfortunately remains.

andywer commented 5 years ago

Thanks for submitting the PR! :)

Yeah, something is severely wrong about that test snapshot... 😅 I will have a look at that.

Thanks everyone for reviewing so quickly. I am terribly busy ATM, but I will find some time to help getting this baby merged 🙌

s-h-a-d-o-w commented 5 years ago

I already addressed that, just didn't push it because I assumed that my changes are useless after all. But... I just read the docs for the paths plugin and found this very interesting bit:

> Notice that the plugin is placed in the resolve.plugins section of the configuration.

So... it seems that this PR actually does address a valid issue. (It of course still begs the question why there haven't been issues with it in the past...)

jvanbruegge commented 5 years ago

I assume because noone used the path resolving. When I created the typescript block, I had no idea what it was or how to set it up, so the result was a best efford

s-h-a-d-o-w commented 5 years ago

Brain fart, please disregard my now deleted last comment. ;)

andywer commented 5 years ago

Any reason not to merge this now? 😉

andywer commented 5 years ago

What's the status here? 🙈

jvanbruegge commented 5 years ago

AFAICT I am using the newest webpack-blocks version with webpack 4 without issues, can check the exact version at home though

andywer commented 4 years ago

Stale and old. Dropped the ball on that one 🙃

Closing for the time being.