Skip to content

Commit

Permalink
fix(core): migrate old invalid glob syntax in target outputs (#18191)
Browse files Browse the repository at this point in the history
(cherry picked from commit fef5b65)
  • Loading branch information
Cammisuli authored and FrozenPandaz committed Jul 20, 2023
1 parent 6b6038c commit 0f3fd9c
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 8 deletions.
4 changes: 2 additions & 2 deletions docs/shared/reference/project-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,13 @@ Sometimes, multiple targets might write to the same directory. When possible it
}
```

But if the above is not possible, globs (parsed with the [minimatch](https://github.com/isaacs/minimatch) library) can be specified as outputs to only cache a set of files rather than the whole directory.
But if the above is not possible, globs (parsed by the [GlobSet](https://docs.rs/globset/0.4.5/globset/#syntax) Rust library) can be specified as outputs to only cache a set of files rather than the whole directory.

```json
{
"targets": {
"build-js": {
"outputs": ["{workspaceRoot}/dist/libs/mylib/**/*.js"]
"outputs": ["{workspaceRoot}/dist/libs/mylib/**/*.{js,map}"]
},
"build-css": {
"outputs": ["{workspaceRoot}/dist/libs/mylib/**/*.css"]
Expand Down
14 changes: 9 additions & 5 deletions e2e/nx-run/src/cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe('cache', () => {
updateProjectConfig(mylib, (c) => {
c.targets.build = {
executor: 'nx:run-commands',
outputs: ['{workspaceRoot}/dist/*.txt'],
outputs: ['{workspaceRoot}/dist/*.{txt,md}'],
options: {
commands: [
'rm -rf dist',
Expand All @@ -167,7 +167,8 @@ describe('cache', () => {
'echo c > dist/c.txt',
'echo d > dist/d.txt',
'echo e > dist/e.txt',
'echo f > dist/f.txt',
'echo f > dist/f.md',
'echo g > dist/g.html',
],
parallel: false,
},
Expand All @@ -188,7 +189,8 @@ describe('cache', () => {
expect(outputsWithUntouchedOutputs).toContain('c.txt');
expect(outputsWithUntouchedOutputs).toContain('d.txt');
expect(outputsWithUntouchedOutputs).toContain('e.txt');
expect(outputsWithUntouchedOutputs).toContain('f.txt');
expect(outputsWithUntouchedOutputs).toContain('f.md');
expect(outputsWithUntouchedOutputs).toContain('g.html');

// Create a file in the dist that does not match output glob
updateFile('dist/c.ts', '');
Expand All @@ -202,7 +204,8 @@ describe('cache', () => {
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('c.txt');
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('d.txt');
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('e.txt');
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('f.txt');
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('f.md');
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('g.html');
expect(outputsAfterAddingUntouchedFileAndRerunning).toContain('c.ts');

// Clear Dist
Expand All @@ -217,8 +220,9 @@ describe('cache', () => {
expect(outputsWithoutOutputs).toContain('c.txt');
expect(outputsWithoutOutputs).toContain('d.txt');
expect(outputsWithoutOutputs).toContain('e.txt');
expect(outputsWithoutOutputs).toContain('f.txt');
expect(outputsWithoutOutputs).toContain('f.md');
expect(outputsWithoutOutputs).not.toContain('c.ts');
expect(outputsWithoutOutputs).not.toContain('g.html');
});

it('should use consider filesets when hashing', async () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/nx/migrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@
"version": "16.2.0-beta.0",
"description": "Remove outputPath from run commands",
"implementation": "./src/migrations/update-16-2-0/remove-run-commands-output-path"
},
"16.5.4-update-output-globs": {
"cli": "nx",
"version": "16.5.4-beta.0",
"description": "Update outdated non-standard globs to unix standard",
"implementation": "./src/migrations/update-16-5-4/update-output-globs"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { createTreeWithEmptyWorkspace } from '../../generators/testing-utils/create-tree-with-empty-workspace';
import { TargetConfiguration } from '../../config/workspace-json-project-json';
import {
addProjectConfiguration,
readProjectConfiguration,
} from '../../generators/utils/project-configuration';
import updateOutputsGlobs from './update-output-globs';
import { readJson, updateJson } from '../../generators/utils/json';
import { NxJsonConfiguration } from '../../config/nx-json';

describe('update output globs', () => {
it('should update output globs', () => {
const tree = createTreeWithEmptyWorkspace();
const targets: Record<string, TargetConfiguration> = {
build: {
outputs: ['{options.outputPath}', 'dist/apps/my-app/*.(js|map|ts)'],
},
lint: {},
test: {
outputs: ['dist/apps/my-app/main.(js|map|ts)'],
},
run: {
outputs: ['dist/apps/my-app'],
},
};
addProjectConfiguration(tree, 'my-app', {
root: 'apps/my-app',
targets,
});

updateJson<NxJsonConfiguration>(tree, 'nx.json', (json) => {
json.targetDefaults = {
lint: {
outputs: ['dist/apps/my-app', '*.(js|map|ts)'],
},
};
return json;
});

updateOutputsGlobs(tree);

const migratedTargets = readProjectConfiguration(tree, 'my-app').targets;
expect(migratedTargets).toMatchInlineSnapshot(`
{
"build": {
"outputs": [
"{options.outputPath}",
"dist/apps/my-app/*.{js,map,ts}",
],
},
"lint": {},
"run": {
"outputs": [
"dist/apps/my-app",
],
},
"test": {
"outputs": [
"dist/apps/my-app/main.{js,map,ts}",
],
},
}
`);

const nxJson = readJson<NxJsonConfiguration>(tree, 'nx.json');
expect(nxJson.targetDefaults).toMatchInlineSnapshot(`
{
"lint": {
"outputs": [
"dist/apps/my-app",
"*.{js,map,ts}",
],
},
}
`);
});
});
54 changes: 54 additions & 0 deletions packages/nx/src/migrations/update-16-5-4/update-output-globs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Tree } from '../../generators/tree';
import {
getProjects,
updateProjectConfiguration,
} from '../../generators/utils/project-configuration';
import { formatChangedFilesWithPrettierIfAvailable } from '../../generators/internal-utils/format-changed-files-with-prettier-if-available';
import { TargetConfiguration } from '../../config/workspace-json-project-json';
import { updateJson } from '../../generators/utils/json';
import { NxJsonConfiguration } from '../../config/nx-json';

function replaceOutput(output: string) {
// replace {projectRoot}/folder/*.(js|map|ts) to {projectRoot}/folder/*.{js,map,ts}
const regex = /\(([^)]+)\)/g;
return output.replace(regex, (match, group1) => {
let replacements = group1.split('|').join(',');
return `{${replacements}}`;
});
}

export default async function updateOutputsGlobs(tree: Tree) {
for (const [projectName, projectConfiguration] of getProjects(
tree
).entries()) {
for (const [targetName, targetConfiguration] of Object.entries(
projectConfiguration.targets ?? {}
)) {
if (!Array.isArray(targetConfiguration.outputs)) {
continue;
}

targetConfiguration.outputs =
targetConfiguration.outputs.map(replaceOutput);
}
updateProjectConfiguration(tree, projectName, projectConfiguration);
}

if (tree.exists('nx.json')) {
updateJson<NxJsonConfiguration>(tree, 'nx.json', (json) => {
for (const [, targetConfiguration] of Object.entries(
json.targetDefaults ?? {}
)) {
if (!Array.isArray(targetConfiguration.outputs)) {
continue;
}

targetConfiguration.outputs =
targetConfiguration.outputs.map(replaceOutput);
}
return json;
});
}

await formatChangedFilesWithPrettierIfAvailable(tree);
}
16 changes: 16 additions & 0 deletions packages/nx/src/native/cache/expand_outputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ mod test {
.child("nx.darwin-arm64.node")
.touch()
.unwrap();
temp.child("multi").child("file.js").touch().unwrap();
temp.child("multi").child("src.ts").touch().unwrap();
temp.child("multi").child("file.map").touch().unwrap();
temp.child("multi").child("file.txt").touch().unwrap();
temp
}
#[test]
Expand All @@ -80,4 +84,16 @@ mod test {
]
);
}

#[test]
fn should_handle_multiple_extensions() {
let temp = setup_fs();
let entries = vec!["multi/*.{js,map,ts}".to_string()];
let mut result = expand_outputs(temp.display().to_string(), entries).unwrap();
result.sort();
assert_eq!(
result,
vec!["multi/file.js", "multi/file.map", "multi/src.ts"]
);
}
}
5 changes: 4 additions & 1 deletion packages/nx/src/tasks-runner/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ export function getOutputsForTargetAndConfiguration(
options,
});
})
.filter((output) => !!output && !output.match(/{.*}/));
.filter(
(output) =>
!!output && !output.match(/{(projectRoot|workspaceRoot|(options.*))}/)
);
}

// Keep backwards compatibility in case `outputs` doesn't exist
Expand Down

0 comments on commit 0f3fd9c

Please sign in to comment.