-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(microfrontends): respect packageName field #10383
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(microfrontends): respect packageName field #10383
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Reviewer's Guide
@@ -394,6 +394,13 @@ impl Engine<Built> { | |||
self.task_graph.node_weights() | |||
} | |||
|
|||
pub fn task_ids(&self) -> impl Iterator<Item = &TaskId<'static>> { |
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.
Future work is removing the concept of a "root" task node since that only exists due to limitation of Go graphing library.
@@ -20,7 +20,8 @@ pub struct MicrofrontendsConfigs { | |||
|
|||
#[derive(Debug, Clone, Default, PartialEq)] | |||
struct ConfigInfo { | |||
tasks: HashSet<TaskId<'static>>, | |||
// A map from tasks declared in the configuration to the application that they belong to | |||
tasks: HashMap<TaskId<'static>, 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.
We need to store the application name as that is what the microfrontends local proxy uses to identify locally running applications.
@@ -269,11 +266,31 @@ impl<'a> CommandProvider for MicroFrontendProxyProvider<'a> { | |||
} | |||
} | |||
|
|||
/// A trait for fetching package information required to execute commands | |||
pub trait PackageInfoProvider { |
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.
This trait allows us to test the microfrontends command generation without building an entire package graph.
Self { | ||
repo_root, | ||
package_graph, | ||
tasks_in_graph, | ||
tasks_in_graph: tasks_in_graph.cloned().collect(), |
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.
Moved this logic over to Engine
so we could test this command provider without building an entire task graph.
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.
Nice
.unwrap() | ||
.unwrap(); | ||
assert!( | ||
cmd.label().ends_with("--names docs-app"), |
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.
Key here is that we pass the "app name" and not the package name
Self { | ||
repo_root, | ||
package_graph, | ||
tasks_in_graph, | ||
tasks_in_graph: tasks_in_graph.cloned().collect(), |
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.
Nice
Co-authored-by: Thomas Knickman <[email protected]>
Description
This PR does 2 things:
microfrontends.json
that is not present in the monorepopackageName
over theapplications
key if presentTesting Instructions
Added unit tests.
Verified warnings now are present if you reference a package that isn't found in the monorepo:
Verified that proxy is now started if every application uses a
packageName
that differs from the key inapplications
.