-
Notifications
You must be signed in to change notification settings - Fork 30
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: don't try to discover shared item version in case of custom version or invalid package name #131
base: main
Are you sure you want to change the base?
Conversation
// 匹配npm包名的正则表达式,忽略路径部分 | ||
const regex = /^(?:@[^/]+\/)?[^/]+/; | ||
const regex = /^(?:@[^/]+\/)?\w[^/]+/; |
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.
A valid package starts with a letter.
console.log(e); | ||
let { version, requiredVersion }: any = typeof shareItem === 'object' ? shareItem : {}; | ||
if (!version) { | ||
const npmPackage = removePathFromNpmPackage(key); |
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.
Aliases probably should take precedence over packages.
options.shared[removePathFromNpmPackage(key)] || | ||
options.shared[removePathFromNpmPackage(key) + '/']; | ||
return shareItem; | ||
return options.shared[key]; |
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.
removePathFromNpmPackage()
doesn't looks to be applied when creating the shared object.
How to configure “shared”? Do Webpack or Rust have this feature? |
Understood, I will review it later. |
Thank you for your contribution, but this feature requires some additional handling, and I will complete it this week. |
There is one more step left for the proxy logic to convert relative paths to absolute paths. I will find time this week to complete it. |
Are there any news on this one? |
I've been really busy lately, working overtime for the weekend and not being able to handle this issue in a timely manner
…---- Replied Message ----
| From | Giorgio ***@***.***> |
| Date | 10/21/2024 17:39 |
| To | module-federation/vite ***@***.***> |
| Cc | zhn ***@***.***>,
Comment ***@***.***> |
| Subject | Re: [module-federation/vite] fix: don't try to discover shared item version in case of custom version or invalid package name (PR #131) |
Are there any news on this one?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: ***@***.***>
|
Maybe shared management should be handled by https://github.com/module-federation/core/blob/main/packages/managers/src/SharedManager.ts |
Yes, unifying the logic is better. You can submit a PR to fix this issue, but implementing shared relative paths will be different from the current process of sharing npm packages. |
I really don’t have time to add this feature, but can use Delegate Modules. |
Exposing a local share causes lots of errors in console: