fix(add-caching): explicitly set targetDefaults for all scripts#3929
fix(add-caching): explicitly set targetDefaults for all scripts#3929fahslaj merged 9 commits intolerna:mainfrom
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 742ce93. 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 8 targets
Sent with 💌 from NxCloud. |
| message: | ||
| "Which scripts need to be run in order? (e.g. before building a project, dependent projects must be built.)\n", | ||
| choices: this.uniqueScriptNames, | ||
| default: existingTargetDefaults, |
There was a problem hiding this comment.
This allows inquirer to pre-select all targets that already have dependsOn configured
| message: | ||
| "Which scripts are cacheable? (Produce the same output given the same input, e.g. build, test and lint usually are, serve and start are not.)\n", | ||
| choices: this.uniqueScriptNames, | ||
| default: existingCacheableOperations, |
There was a problem hiding this comment.
Same as above; this allows inquirer to pre-select all targets that have "cache": true already configured
| nxJson.tasksRunnerOptions = nxJson.tasksRunnerOptions || {}; | ||
| nxJson.tasksRunnerOptions["default"] = nxJson.tasksRunnerOptions["default"] || {}; | ||
| nxJson.tasksRunnerOptions["default"].runner = | ||
| nxJson.tasksRunnerOptions["default"].runner || "nx/tasks-runners/default"; | ||
| nxJson.tasksRunnerOptions["default"].options = nxJson.tasksRunnerOptions["default"].options || {}; | ||
|
|
||
| if (nxJson.tasksRunnerOptions["default"].options.cacheableOperations) { | ||
| this.logger.warn( | ||
| "add-caching", | ||
| "The `tasksRunnerOptions.default.cacheableOperations` property already exists in `nx.json` and will be overwritten by your answers" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Adding the default task runner seems like excess config. Nx should default this for us when this is missing.
The meaningful removal here is of cacheableOperations, which moved to targetDefaults.
| "options": { | ||
| "cacheableOperations": ["build", "test"] | ||
| } | ||
| "runner": "nx/tasks-runners/default" |
There was a problem hiding this comment.
Shouldn’t we remove this here too for simplicity?
| "tasksRunnerOptions": { | ||
| "default": { | ||
| "options": { | ||
| "cacheableOperations": ["build", "test"], |
There was a problem hiding this comment.
This whole config is outdated too, cache directory is set at the top level, no needs for the object
There was a problem hiding this comment.
I've removed this whole section. We're pretty clear that it's Nx doing the caching in the docs and users can find the Nx docs if they need to do advanced things like changing the cache location.
I think we should minimize the amount Nx-specific docs on the Lerna site as they will get out of date quickly.
| "default": { | ||
| "runner": "nx/tasks-runners/default", | ||
| "options": { | ||
| "cacheableOperations": [], |
There was a problem hiding this comment.
Removed. Same idea as the above comment; I removed the whole section to minimize the doc's coupling to Nx configuration.
|
@JamesHenry I've updated the docs you've pointed out. I removed a couple of the outdated sections instead of just updating their config. To reduce the maintenance burden, I think we should keep the more advanced Nx configuration options to the Nx docs and link to the Nx site instead of duplicating them in the Lerna docs. |
Description
targetDefaultsfor all scripts, not just those selected in the prompts.Motivation and Context
This allows
lerna runto correctly identify when the user has configured Nx vialerna add-caching, even if they didn't select any scripts to run in order. This also fixesadd-cachingfor Nx 17.How Has This Been Tested?
This has been tested manually on a fresh workspace.
Types of changes
Checklist: