fix(repo): add copy-assets plugin and migrate all packages from legacy-post-build#34994
fix(repo): add copy-assets plugin and migrate all packages from legacy-post-build#34994FrozenPandaz merged 42 commits intomasterfrom
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit 90d2206
☁️ Nx Cloud last updated this comment at |
7eddc97 to
fa911c7
Compare
776b85f to
f3dd1c3
Compare
c5db95e to
e7c15ca
Compare
| ? [ | ||
| `${relative(projectRoot, assetsJson.outDir)}/**`, |
There was a problem hiding this comment.
The relative() call will fail when assetsJson.outDir is an absolute path. The code assumes assetsJson.outDir is always relative to projectRoot, but several assets.json files use absolute-style paths (e.g., packages/nx/dist, packages/dotnet/dist). When assetsJson.outDir doesn't start with projectRoot, relative() produces paths with .. segments that should not be in the ignore pattern.
const ignore =
input === projectRoot && !path.isAbsolute(assetsJson.outDir)
? [
`${relative(projectRoot, assetsJson.outDir)}/**`,
...(asset.ignore ?? []),
]
: asset.ignore;| ? [ | |
| `${relative(projectRoot, assetsJson.outDir)}/**`, | |
| input === projectRoot && !path.isAbsolute(assetsJson.outDir) | |
| ? [ | |
| `${relative(projectRoot, assetsJson.outDir)}/**`, |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
a3a8def to
4571187
Compare
| const outputs = normalized.map((entry) => { | ||
| const outputPath = getAssetOutputPath(entry.pattern, entry); | ||
| if (outputPath.startsWith(projectRoot + '/')) { | ||
| return `{projectRoot}/${outputPath.slice(projectRoot.length + 1)}`; | ||
| } | ||
| return `{workspaceRoot}/${outputPath}`; | ||
| }); |
There was a problem hiding this comment.
The getAssetOutputPath function is being called with entry.pattern (a glob pattern like packages/angular/**/files/**) but it expects an actual file path as its first argument. This will produce incorrect output patterns.
The function computes path.relative(entry.input, src) where src is expected to be a file path, not a glob pattern. For glob patterns, this will produce unpredictable results.
To fix, the outputs should be constructed directly from entry.output without trying to compute them from patterns:
const outputs = normalized.map((entry) => {
if (entry.output.startsWith(projectRoot + '/')) {
return `{projectRoot}/${entry.output.slice(projectRoot.length + 1)}/**`;
}
return `{workspaceRoot}/${entry.output}/**`;
});| const outputs = normalized.map((entry) => { | |
| const outputPath = getAssetOutputPath(entry.pattern, entry); | |
| if (outputPath.startsWith(projectRoot + '/')) { | |
| return `{projectRoot}/${outputPath.slice(projectRoot.length + 1)}`; | |
| } | |
| return `{workspaceRoot}/${outputPath}`; | |
| }); | |
| const outputs = normalized.map((entry) => { | |
| if (entry.output.startsWith(projectRoot + '/')) { | |
| return `{projectRoot}/${entry.output.slice(projectRoot.length + 1)}/**`; | |
| } | |
| return `{workspaceRoot}/${entry.output}/**`; | |
| }); | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
c2e1ab8 to
8024262
Compare
…puts Add outputs to maven-batch-runner _package target so dependentTasksOutputFiles can resolve the jar files. Narrow workspace-plugin input to just the executor files.
The !(*.ts) extglob doesn't work correctly in Nx inputs, causing .d.ts files to be excluded. Replace with explicit globs matching the standard pattern used by other packages.
Change **/tsconfig*.json to tsconfig*.json in asset ignores so template tsconfig files inside files/ directories are still copied.
Remove tsConfig, main, types, packageRoot, and addPackageJsonFields options. The executor now only needs assets and outputPath since the plugin always provides both.
The old legacy-post-build executor copied typings to types at build time. Now that the executor is simplified, set types directly in source package.json and remove the deprecated typings field.
…-specific cache Split .node files into separate asset entry with includeIgnoredFiles so they are tracked via dependentTasksOutputFiles. Add build-native to copy-assets dependsOn so the cache key includes the platform.
e164c3f to
8309fd7
Compare
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
🎓 Learn more about Self-Healing CI on nx.dev
b440466 to
90d2206
Compare
| try { | ||
| await assetHandler.processAllAssetsOnce(); | ||
| } catch (error) { | ||
| logger.error(`Error processing assets: ${error.message || error}`); |
There was a problem hiding this comment.
Nit: if error: unknown then you'd to check instanceof Error or that message is a property because using. We don't have strict mode on so it's passing typechecks.
## Current Behavior - The copy-assets plugin copies `assets.json` into the output directory (it doesn't exclude itself) - The copy-assets executor catches errors with `error.message` but `error` is typed as `unknown`, which can fail at runtime - No way to skip e2e cleanup when debugging locally ## Expected Behavior - `assets.json` is excluded from being copied into the output directory - Error handling properly checks `instanceof Error` before accessing `.message` - Setting `NX_E2E_SKIP_CLEANUP=true` preserves the test project directory for debugging ## Related Issue(s) Follow-up to #34994 — addresses review comments from that PR. --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com>
|
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
Each package defines a
legacy-post-buildtarget inproject.jsonwith inline asset copy configuration. Inputs are not accurately declared, leading to sandbox violations in CI. The asset globs, ignores, and outputs must be manually kept in sync across 37 packages.Expected Behavior
A
copy-assetscreateNodesV2 plugin readsassets.jsonfrom each package and automatically generates the target with:CopyAssetsHandlerdependentTasksOutputFilesfor gitignored build artifacts (jars, native binaries)outDirexclusion from asset copiesChanges
New infrastructure:
copy-assetscreateNodesV2 plugin intools/workspace-plugincopy-assetsexecutor (simplified fromlegacy-post-build— just copies assets, no package.json field manipulation)normalizeAssetsandgetAssetOutputPathinto reusable module inpackages/jscopyReadmenamedInput for copy-readme build targetsMigration (all 37 packages):
assets.jsonfor every package defining what to copylegacy-post-buildtargets from project.json fileslegacy-post-buildtarget defaults from nx.jsoncopy-local-nativetarget (replaced by.node/.wasmin asset glob)Cleanup:
creator-filesglobs, non-existent template files, typo'd directory namestsconfig*.jsonignore instead of recursive (so template tsconfigs infiles/dirs are still copied)!(*.ts)extglob patterns with explicit globs (extglobs don't work correctly in Nx inputs)buildTargets: ["build-base"]andtslibdependency_packagetarget for correctdependentTasksOutputFilesresolutionOther fixes:
.swcdirectories from sandbox write checksangular-rspackpackages (removeaddTypecheckTarget: false)@nx/pluginto root package.json