-
-
Notifications
You must be signed in to change notification settings - Fork 620
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
feat!: align AssetGeneratorDataUrlFunction with webpack #8614
Conversation
✅ Deploy Preview for rspack ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about be compatible with the previous API signature to avoid breaking the user's code?
For example, if arg
is an object, convert it to source + context
.
We can align type declarations with webpack, but the internal implementation is compatible with Rspack <= 1.1.5. The compatible code can be removed in Rspack v2.0.
Sorry for the delay. If the parameters were directly passed to us by the user, we could maintain compatibility with the old version. However, in this case, the user provides a dataUrl function, and we pass the parameters to it, making compatibility with the old version unfeasible. Let’s put this PR on hold for now and merge it at a more suitable time. |
This is not a very common usage, and it makes more sense to align with webpack. Maybe we can include this change in Rspack v1.2.0. @hardfist what do you think |
8f987a1
to
750fc12
Compare
@hardfist cc |
After discussion, we decided to include this PR in v1.2.0 to align with webpack. @inottn Can you rebase the PR again~ ❤️ |
750fc12
to
b5c2f02
Compare
done~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Summary
close #8538
This is a breaking change, but the previous behavior was inconsistent with webpack.
Checklist