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 webpack deprecations (fix #1126) #1135

Merged
merged 8 commits into from
Jul 15, 2020
Merged

Fix webpack deprecations (fix #1126) #1135

merged 8 commits into from
Jul 15, 2020

Conversation

g-plane
Copy link
Contributor

@g-plane g-plane commented Jul 1, 2020

This PR should remove some of webpack deprecation warnings.

@johnnyreilly
Copy link
Member

@sheetalkamat has a PR which drops support for older versions of TypeScript here: #1136

When that's merged we can take a look at your PR - it only seems to be failing on older versions of TypeScript.

@g-plane
Copy link
Contributor Author

g-plane commented Jul 9, 2020

It seems that these are weird failures, and I'm not sure whether upgrading webpack will fix these or not, because webpack 4.37 (and newer versions) will check the current loader context is null or not.

@johnnyreilly
Copy link
Member

Very strange....

@g-plane
Copy link
Contributor Author

g-plane commented Jul 10, 2020

I've confirmed that upgrading webpack will solve the failures in CI, as those failures also can be reproduced on my local computer. Can I open another PR to upgrade webpack? If not, we may need to add some hacky code to check the loader context.

@johnnyreilly
Copy link
Member

Yeah sure - I'd like to decouple the upgrade of webpack from this change if that's okay. So upgrade webpack first and loop back to this next.

@g-plane
Copy link
Contributor Author

g-plane commented Jul 11, 2020

PR for upgrading webpack is created: #1140 .

@g-plane g-plane marked this pull request as ready for review July 13, 2020 05:53
@g-plane
Copy link
Contributor Author

g-plane commented Jul 13, 2020

I've marked this PR as ready for review, however I don't know how to address the problems at AppVeyor.

@johnnyreilly
Copy link
Member

AppVeyor has had a bad day - let's see if it fixes up now.

@g-plane
Copy link
Contributor Author

g-plane commented Jul 13, 2020

Maybe we can migrate to GitHub Actions in the future.

@johnnyreilly
Copy link
Member

We're already using GitHub actions, but it doesn't cover the use case that AppVeyor provides; namely running CI on Windows.

Maybe Azure pipelines has something to offer - I haven't taken a look in a while. I did start looking here though:

#856

Copy link
Member

@johnnyreilly johnnyreilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! Do you want to increment the package.json and add an entry to the CHANGELOG.md?

@g-plane
Copy link
Contributor Author

g-plane commented Jul 14, 2020

I will. By the way, GitHub Actions supports Linux, Windows and macOS.

@g-plane g-plane changed the title Fix webpack deprecations Fix webpack deprecations (fix #1126) Jul 14, 2020
@johnnyreilly
Copy link
Member

Really? Did not know that! That said, I never managed to get comparison tests passing with GitHub actions. But that was the early days

@g-plane
Copy link
Contributor Author

g-plane commented Jul 14, 2020

Maybe we can try it after this PR.

@g-plane
Copy link
Contributor Author

g-plane commented Jul 14, 2020

I've updated package.json and changelog. Is this OK?

@johnnyreilly
Copy link
Member

Looks good! Let's wait for Travis to get happy then we can merge

@johnnyreilly
Copy link
Member

johnnyreilly commented Jul 14, 2020

My feelings on AppVeyor right now

@g-plane
Copy link
Contributor Author

g-plane commented Jul 14, 2020

All passed now.

@johnnyreilly johnnyreilly merged commit abae5fd into TypeStrong:master Jul 15, 2020
@johnnyreilly
Copy link
Member

Shipping https://github.com/TypeStrong/ts-loader/releases/tag/v8.0.1 - thanks for your work!

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.

2 participants