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: ignoreWarnings not working on webpack 5 #91

Merged
merged 8 commits into from
May 27, 2024

Conversation

ImLunaHey
Copy link

@ImLunaHey ImLunaHey commented Oct 17, 2023

if this PR isn't wanted im happy to maintain my own fork. 🙏

also im not 100% how to fix the error the tests are throwing. 🤔
if someone can point me in the right direction ill be happy to fix them up.

closes: #90 and #81

Copy link
Owner

@RoccoC RoccoC left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

A few considerations:

  • technically this is a breaking change since the plugin will no longer work with webpack versions < 5, so we should bump the version to 3.0.0
  • the failing test is due to changes in the postcss-loader config API, but after fixing this I found that the original test case is no longer valid for webpack 5 and the latest version of autoprefixer. I think it's best to remove the Should handle warning from child compiler test case from tests/test.spec.ts, as well as any related files that are no longer needed for the test (tests/childWarning.js, tests/test.css). This means we can also remove the css loaders/plugins from tests/webpack.config.ts and package dependencies, too.
  • I know that TSLint is long deprecated, but I think it would be great to replace it with ESLint (or at least keep it around until we do).

Thanks again! 🙏

package.json Outdated Show resolved Hide resolved
@ImLunaHey
Copy link
Author

Let me get this fixed up. 🙏

@ImLunaHey
Copy link
Author

@RoccoC let me know if that resolves all the issues. 🙏

@ImLunaHey
Copy link
Author

bump ⬆️

package.json Show resolved Hide resolved
@RoccoC
Copy link
Owner

RoccoC commented Nov 17, 2023

bump ⬆️

There's an issue when running npm test, but otherwise all looks good! Thanks again, and so sorry for the delay!

@ImLunaHey
Copy link
Author

if tests need to pass before merging then a CI action should be added to enforce that.

@ImLunaHey
Copy link
Author

i cannot get the tests to run locally. they just sit there without doing anything.

➜  webpack-build-notifier git:(master) ✗ npm run test -- --verbose

> webpack-build-notifier@3.0.0 test
> jest --verbose


 RUNS  tests/test.spec.ts

@RoccoC RoccoC changed the base branch from master to fix-ignore-warnings May 27, 2024 16:59
@RoccoC RoccoC merged commit fafc00f into RoccoC:fix-ignore-warnings May 27, 2024
@RoccoC
Copy link
Owner

RoccoC commented May 27, 2024

i cannot get the tests to run locally. they just sit there without doing anything.

I'll get these fixed up and merged. Thanks for your PR.

RoccoC added a commit that referenced this pull request May 27, 2024
@RoccoC
Copy link
Owner

RoccoC commented May 27, 2024

if tests need to pass before merging then a CI action should be added to enforce that.

Thanks for flagging this. Looks like TravisCI was no longer working. I went ahead and migrated to CircleCI.

@chadlwilson
Copy link

Thanks for the work here!

Just to confirm - given the major version bump, does this drop support for Webpack 4 or still backwards compatible?

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.

webpacks's ignoreWarnings is not used
3 participants