-
Notifications
You must be signed in to change notification settings - Fork 400
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: passing BindingResolveOptions#alias
from js to rust with maintaining order
#798
fix: passing BindingResolveOptions#alias
from js to rust with maintaining order
#798
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
CodSpeed Performance ReportMerging #798 will not alter performanceComparing Summary
|
#[serde(rename_all = "camelCase")] | ||
pub struct AliasItem { | ||
pub find: String, | ||
pub replacement: Vec<String>, |
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.
pub replacement: Vec<String>, | |
pub replacements: Vec<String>, |
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.
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.
filename should be alias_item.rs
.
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.
Done
export type RolldownResolveOptions = Omit<BindingResolveOptions, 'alias'> & { | ||
alias?: Record<string, string> | ||
alias?: Record<string, string> | Alias[] |
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.
Please don't change the type of alias
. For users, they still pass alias using object.
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.
But I notice that, in vite
and rollup
, config alias
is support for Array and object
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.
Yeah. Vite use https://github.com/rollup/plugins/tree/master/packages/alias#entries under the hood. The API it provided has different usages for array
or object
, which means you could not use object
to use customResolver
API.
However, we don't have this need. In our cases, object
and array
does the same thing. To keep it simple, supporting object
only would be fine.
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.
Ok, get it
BindingResolveOptions#alias
from js to rust with maintaining order
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.
Thanks
78d84cd
to
d27f635
Compare
Description
convert
alias
toVec<AliasItem>
for issue #790