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

feat(storybook-addon): refactor util withModuleFederation #829

Conversation

fyodorovandrei
Copy link
Contributor

@fyodorovandrei fyodorovandrei commented Apr 28, 2023

I opened a PR in NX repo to avoid passing a function to determinate the remote URL
for util getModuleFederationConfig. The PR is merged and now we can refactor util withModuleFederation.

@@ -10,6 +10,8 @@

[module-federation/typescript](./packages/typescript)

[module-federation/storybook-addon](./packages/storybook-addon)

## Generate an application

Run `nx g @nrwl/next:app my-app` to generate an application.
Copy link

Choose a reason for hiding this comment

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

It seems that this is not actually code but rather a markdown document, likely related to a project's documentation or readme file.

As for the content itself, it looks like a brief introduction to a project, specifically mentioning its use of Nx and Next.js. There is also a note about generating an application using the nx g command.

Without additional context, it is difficult to provide specific feedback or improvements. However, listing out the changes made in the code patch would be helpful in understanding any potential risks or opportunities for improvement.

@@ -122,7 +121,7 @@
"merge-stream": "~2.0",
"ngx-deploy-npm": "^5.0.0",
"nodejieba": "^2.6.0",
"nx": "15.9.2",
"nx": "16.0.0",
"postcss-calc": "~8.2.0",
"postcss-custom-properties": "~13.1.0",
"postcss-import": "~15.1.0",
Copy link

Choose a reason for hiding this comment

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

The code patch appears to be a dependency update in the package.json file. It updates various @nrwl packages and the nx package to version 16.0.0. There are no apparent syntax or logic issues in the code patch, and it seems to be a routine package update. However, it is always recommended to test thoroughly after updating dependencies to ensure compatibility and stability.

@@ -1,6 +1,6 @@
{
"name": "@module-federation/storybook-addon",
"version": "0.1.0",
"version": "0.1.1",
"description": "Storybook addon to consume remote module federated apps/components",
"license": "MIT",
"repository": "https://github.com/module-federation/universe/tree/main/packages/storybook-addon",
Copy link

Choose a reason for hiding this comment

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

This is not a code patch, but rather a patch file containing changes to a package.json file.

The only change in this patch is the version number, which has been updated from "0.1.0" to "0.1.1". There are no bug risks associated with this change and it is a common practice to update the version number when making changes to a package.

Improvement suggestions would depend on the context of this package and its usage, but without more information, it is difficult to provide specific suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

my release system automatically incrementes package versions

@ScriptedAlchemy
Copy link
Member

Y@rn again. Lockfile is out of date

@ScriptedAlchemy ScriptedAlchemy merged commit 5eeab46 into module-federation:main Apr 29, 2023
@fyodorovandrei fyodorovandrei deleted the features/refactor-with-module-federation-util branch August 25, 2023 08:50
RussellCanfield pushed a commit to RussellCanfield/nextjs-mf that referenced this pull request Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants