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(dev-server): prebundle webpack-dev-server and pin webpack-dev-middleware #7788

Closed
wants to merge 7 commits into from

Conversation

hardfist
Copy link
Contributor

@hardfist hardfist commented Sep 4, 2024

Summary

related to #7782 and #7676

new published webpack-dev-server and webpack-dev-middleware can easily break rspack-cli cause there're no tests in webpack-dev-server & webpack-dev-middle repo and rspack-dev-server use some internal apis of these two packages.

so we need to pin webpack-dev-server & webpack-dev-middleware version to avoid accidentally breaking,
but since webpack-dev-middleware is indirect dependency of webpack-dev-server, there're no easy way to pin it without bundling webpack-dev-server.

This pr is a temporarily solution, and the long term solutions is refactor rspack-dev-server implementation and maybe add tests in webpack-dev-server and webpack-dev-middleware about rspack compatibility

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Sep 4, 2024
@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Sep 4, 2024
Copy link

netlify bot commented Sep 4, 2024

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit 8def56d
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/66d912edaf03c50008229b1b
😎 Deploy Preview https://deploy-preview-7788--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hardfist
Copy link
Contributor Author

hardfist commented Sep 4, 2024

@alexander-akait is it acceptable that we add rspack compat tests in webpack-dev-server and webpack-dev-middleware repos or any other suggestions?

@hardfist hardfist force-pushed the yj/prebundle-webpack-dev-server branch from fb9e987 to 581188b Compare September 4, 2024 03:55
Copy link
Member

@SoonIter SoonIter left a comment

Choose a reason for hiding this comment

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

require.resolve is not modified by tsc-alias

clientImplementation = require.resolve(

chenjiahan
chenjiahan previously approved these changes Sep 4, 2024
@chenjiahan chenjiahan dismissed their stale review September 4, 2024 08:31

need to resolve require.resolve issue

@hardfist
Copy link
Contributor Author

hardfist commented Sep 5, 2024

close since this is too complex and hacky, waiting for rspack-dev-server refactor

@hardfist hardfist closed this Sep 5, 2024
@alexander-akait
Copy link

@hardfist what is broken, I think we can fix them

@hardfist
Copy link
Contributor Author

hardfist commented Sep 9, 2024

@hardfist what is broken, I think we can fix them

@alexander-akait thanks, it' broken cause rspack-dev-server use some private api https://github.com/web-infra-dev/rspack/blob/main/packages/rspack-dev-server/src/server.ts#L159-L174 of webpack-dev-server(we're refactoring the rspack-dev-server to use the public api only #7794), so it's actually nothing wrong with webpack-dev-server itself, but it would be nice if we can test the new version of webpack-dev-server compat before users are hitting the problems.

@alexander-akait
Copy link

@hardfist I am fine with adding cross tests 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants