-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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(misc): non conflicting init/add flow #22791
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 2cfe216. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
a41e685
to
bb8c8a1
Compare
714cbaa
to
1bcacf3
Compare
3a3e95c
to
f5d937b
Compare
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.
Looks good, one minor nitpick that may be worth changing
|
||
runInstall(repoRoot, pmc); | ||
|
||
output.log({ title: '🔨 Configuring plugins' }); |
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.
output.log({ title: '🔨 Configuring plugins' }); | |
if (plugins.length) | |
output.log({ title: '🔨 Configuring plugins' }); |
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
When initializing a plugin, the same target name is always chosen. Sometimes, this may conflict with existing targets. For example, if you already have a
build
npm script, as a build target, then thebuild
target coming in from thevite
plugin overrides it. This may cause unexpected behavior resulting from adding the plugin. Already in the current version, we combat this by not includingnpm
scripts as targets. However, for targets likebuild
, thenpm
script is likely what people are already considering their defactobuild
script.Expected Behavior
Plugins are initialized with a set of possible options. The option which does not conflict with the current graph is chosen. If none of the provided options does not conflict, an error is thrown. The plugins prefer the shorter more general target name such as
build
however, will shift to target names such asvite:build
for thevite plugin
.Because plugins now ensure that they don't conflict with the existing graph, we can now add
npm
scripts by default. As such, thenx init
flow has also been updated to add thosenpm
scripts. This results in annx init
flow which is more incremental. You can now start atpackage-based
with no plugins and add plugins afterwards with no worry of plugins altering the existing project graph. In addition, plugins are not selected by default in the init flow as they are easier to add later.https://nx-dev-git-fork-frozenpandaz-init-lite-nrwl.vercel.app/recipes/adopting-nx/adding-to-monorepo
Related Issue(s)
Fixes #