-
Notifications
You must be signed in to change notification settings - Fork 29
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: try to find shared package.json #216
base: main
Are you sure you want to change the base?
fix: try to find shared package.json #216
Conversation
This solution is working for me!
while (path.parse(currentDir).base !== 'node_modules' && currentDir !== path.parse(currentDir).root)
|
Hi @irwinarruda Finally, for the third point. Do you use module federation in node contexte ? |
The problem we had was because In your implementation, the For some reason, I had those Browserify packages for emulating Node Apis in the browser. I don't think I even use them. Either way, for those libs, the |
I also think #131 will affect this PR if it is merged. |
@irwinarruda It looks like you've carefully debugged the change, thanks
This PR is stale, we can merge this one an than try to complete that one too. |
Hi @irwinarruda |
currentDir = path.dirname(currentDir); | ||
} | ||
|
||
throw new Error(`Unable to find package.json for the module "${moduleName}"`); |
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.
I like it @nicotu01 👍
A good error message is always valuable
it looks good to me. |
It's also working for me. Thank you!
I'm sure it's because of the console.log("browserify", require.resolve("browserify"));
console.log("process", require.resolve("process"));
console.log("buffer", require.resolve("buffer"));
// browserify /Users/irwinarruda/Documents/project/node_modules/.pnpm/[email protected]/node_modules/browserify/index.js
// process process
// buffer buffer The good thing now is that the code throws an error instead of getting the wrong
I think if we try to fit those packages into this implementation, we'll have to change everything... In my case, those were used by Webpack and should've been in devDependencies. |
After I sent the previous message, I tested the code in another MF project. The Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /Users/irwinarruda/Project/node_modules/firebase/package.json Firebase does not have a main entry file. From what I understand, it's only meant to be used by its exports, such as There is a |
Yes, there's something wrong. After some investigations on webpack federation plugin.
I think the solution is to find package.json version from something that can be imported (resolved). But the function
So, can we remove the usage of the function |
I couldn't think of any other packages with the same behavior. |
Yes, I think too. I think This function is no more useful since we find the package.json after resolve |
In that file is used only there, so yes. |
|
Make sure the tests pass, and you’re welcome to merge. |
Hi @zhangHongEn So, You can share You can also share But you can't share Here is the exports from react package:
|
For my use case, it's all working fine... However, there is an error using federation({
name: 'host',
manifest: true,
filename: 'remoteEntry.js',
shared: {
'react/': { singleton: true },
'react-dom': { singleton: true },
},
}),
I believe it's an error in the |
This issue is starting to get more complex than I've imagined... I don't know if it's the best one, but the easiest solution is to keep function getNormalizeShareItem(key) {
var options = getNormalizeModuleFederationOptions();
var shareItem =
options.shared[cleanShareItem(key)] ??
options.shared[removePathFromNpmPackage(key)] ??
options.shared[removePathFromNpmPackage(key) + '/'];
return shareItem;
} |
@irwinarruda thanks for your commitment, I think we can ship a solution and then iterate |
@irwinarruda I push a new commit for your usecase. |
Fix #152