-
Notifications
You must be signed in to change notification settings - Fork 148
Filter out node_modules without breaking nx monorepos #352
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
base: main
Are you sure you want to change the base?
Conversation
@@ -191,6 +193,14 @@ async function generateBundleYaml( | |||
frameworkVersion: nextVersion, | |||
}, | |||
}; | |||
if (!process.env.MONOREPO_COMMAND) { |
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.
What will be the mechanism for handling output files when it is a monorepo then, does this mean we do not delete any files without serverApp being set?
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.
Nx monorepos at least will behave as they've always behaved in the past, nothing is deleted
@@ -191,6 +193,14 @@ async function generateBundleYaml( | |||
frameworkVersion: nextVersion, | |||
}, | |||
}; | |||
if (!process.env.MONOREPO_COMMAND) { |
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 think proper behavior is that we support monorepo projects as well.
But I can see how that's a hassle now and requires some careful rethinking.
can we leave a TODO here linking to a bug?
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 can create a bug and link it here to support nx monorepos properly. I don't have much experience there right now so I don't know immediately how to solve this issues since the standalone build doesn't come with a node_modules for some reason but adding the todo will be a good reminder for later anyways
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.
approving with correct account.
Round 2 of filtering out node_modules, this time with a carve out to avoid breaking monorepos