Skip to content

Commit

Permalink
chore(repo): review feedback
Browse files Browse the repository at this point in the history 
Co-authored-by: Giora Guttsait <giora111@gmail.com>
  • Loading branch information
@AgentEnder @gioragutt
AgentEnder and gioragutt committed May 26, 2022
1 parent 1f0c162 commit c3e5993
Show file tree
Hide file tree
Showing 11 changed files with 169 additions and 52 deletions.
8 changes: 7 additions & 1 deletion docs/generated/devkit/index.md
View file
Expand Up @@ -433,7 +433,13 @@ A plugin for Nx

### TargetConfiguration

• **TargetConfiguration**: `Object`
• **TargetConfiguration**<`T`\>: `Object`

#### Type parameters

| Name | Type |
| :--- | :---- |
| `T` | `any` |

---

Expand Down
2 changes: 1 addition & 1 deletion docs/generated/packages/devkit.json
View file

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion docs/generated/packages/nx-plugin.json
View file
Expand Up @@ -329,7 +329,8 @@
"properties": {
"projectName": {
"type": "string",
"description": "Which project should be the target be added to?"
"description": "Which project should be the target be added to?",
"$default": { "$source": "projectName" }
}
},
"required": ["projectName"],
Expand Down
35 changes: 31 additions & 4 deletions e2e/nx-plugin/src/nx-plugin.test.ts
View file
Expand Up @@ -189,6 +189,9 @@ describe('Nx Plugin', () => {
const plugin = uniq('plugin');
const goodGenerator = uniq('good-generator');
const goodExecutor = uniq('good-executor');
const goodMigration = uniq('good-migration');
const badMigrationVersion = uniq('bad-version');
const missingMigrationVersion = uniq('missing-version');

// Generating the plugin results in a generator also called {plugin},
// as well as an executor called "build"
Expand All @@ -202,11 +205,22 @@ describe('Nx Plugin', () => {
`generate @nrwl/nx-plugin:executor ${goodExecutor} --project=${plugin}`
);

runCLI(
`generate @nrwl/nx-plugin:migration ${badMigrationVersion} --project=${plugin} --packageVersion="invalid"`
);

runCLI(
`generate @nrwl/nx-plugin:migration ${missingMigrationVersion} --project=${plugin} --packageVersion="0.1.0"`
);

runCLI(
`generate @nrwl/nx-plugin:migration ${goodMigration} --project=${plugin} --packageVersion="0.1.0"`
);

updateFile(`libs/${plugin}/generators.json`, (f) => {
const json = JSON.parse(f);
// @proj/plugin:plugin has an invalid implementation path
json.generators[plugin].factory =
json.generators[plugin].factory.substring(1);
json.generators[plugin].factory = `./generators/${plugin}/bad-path`;
// @proj/plugin:non-existant has a missing implementation path amd schema
json.generators['non-existant-generator'] = {};
return JSON.stringify(json);
Expand All @@ -215,13 +229,18 @@ describe('Nx Plugin', () => {
updateFile(`libs/${plugin}/executors.json`, (f) => {
const json = JSON.parse(f);
// @proj/plugin:build has an invalid implementation path
json.executors['build'].implementation =
json.executors['build'].implementation.substring(1);
json.executors['build'].implementation = './executors/build/bad-path';
// @proj/plugin:non-existant has a missing implementation path amd schema
json.executors['non-existant-executor'] = {};
return JSON.stringify(json);
});

updateFile(`libs/${plugin}/migrations.json`, (f) => {
const json = JSON.parse(f);
delete json.generators[missingMigrationVersion].version;
return JSON.stringify(json);
});

const results = runCLI(`lint ${plugin}`, { silenceError: true });
expect(results).toContain(
`${plugin}: Implementation path should point to a valid file`
Expand All @@ -244,6 +263,14 @@ describe('Nx Plugin', () => {
`non-existant-executor: Missing required property - \`implementation\``
);
expect(results).not.toContain(goodExecutor);

expect(results).toContain(
`${missingMigrationVersion}: Missing required property - \`verison\``
);
expect(results).toContain(
`${badMigrationVersion}: Version should be a valid semver`
);
expect(results).not.toContain(goodMigration);
});

describe('local plugins', () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/eslint-plugin-nx/src/index.ts
View file
Expand Up @@ -11,9 +11,9 @@ import enforceModuleBoundaries, {
RULE_NAME as enforceModuleBoundariesRuleName,
} from './rules/enforce-module-boundaries';

import nxPluginSchemaRule, {
RULE_NAME as nxPluginSchemaRuleName,
} from './rules/nx-plugin-schema';
import nxPluginChecksRule, {
RULE_NAME as nxPluginChecksRuleName,
} from './rules/nx-plugin-checks';

// Resolve any custom rules that might exist in the current workspace
import { workspaceRules } from './resolve-workspace-rules';
Expand All @@ -31,7 +31,7 @@ module.exports = {
},
rules: {
[enforceModuleBoundariesRuleName]: enforceModuleBoundaries,
[nxPluginSchemaRuleName]: nxPluginSchemaRule,
[nxPluginChecksRuleName]: nxPluginChecksRule,
...workspaceRules,
},
};
61 changes: 48 additions & 13 deletions ...t-plugin-nx/src/rules/nx-plugin-schema.ts → ...t-plugin-nx/src/rules/nx-plugin-checks.ts
View file
Expand Up @@ -43,7 +43,7 @@ export type MessageIds =
| 'noExecutorsOrBuildersFound'
| 'valueShouldBeObject';

export const RULE_NAME = 'nx-plugin-schema';
export const RULE_NAME = 'nx-plugin-checks';

export default createESLintRule<Options, MessageIds>({
name: RULE_NAME,
Expand All @@ -59,8 +59,8 @@ export default createESLintRule<Options, MessageIds>({
invalidImplementationPath:
'{{ key }}: Implementation path should point to a valid file',
invalidImplementationModule:
'Unable to find export {{ identifier }} in implementation module',
invalidVersion: 'Version should be a valid semver',
'{{ key }}: Unable to find export {{ identifier }} in implementation module',
invalidVersion: '{{ key }}: Version should be a valid semver',
noGeneratorsOrSchematicsFound:
'Unable to find `generators` or `schematics` property',
noExecutorsOrBuildersFound:
Expand All @@ -69,7 +69,7 @@ export default createESLintRule<Options, MessageIds>({
missingRequiredSchema: '{{ key }}: Missing required property - `schema`',
missingImplementation:
'{{ key }}: Missing required property - `implementation`',
missingVersion: 'Missing required property: `version`',
missingVersion: '{{ key }}: Missing required property - `version`',
},
},
defaultOptions: [DEFAULT_OPTIONS],
Expand Down Expand Up @@ -290,7 +290,7 @@ export function validateEntry(
node: baseNode as any,
});
} else {
validateImplemenationNode(implementationNode, context);
validateImplemenationNode(implementationNode, key, context);
}

if (mode === 'migration') {
Expand All @@ -300,6 +300,9 @@ export function validateEntry(
if (!versionNode) {
context.report({
messageId: 'missingVersion',
data: {
key,
},
node: baseNode as any,
});
} else if (
Expand All @@ -308,13 +311,19 @@ export function validateEntry(
) {
context.report({
messageId: 'invalidVersion',
data: {
key,
},
node: versionNode.value as any,
});
} else {
const specifiedVersion = versionNode.value.value;
if (!valid(specifiedVersion)) {
context.report({
messageId: 'invalidVersion',
data: {
key,
},
node: versionNode.value as any,
});
}
Expand All @@ -324,6 +333,7 @@ export function validateEntry(

export function validateImplemenationNode(
implementationNode: AST.JSONProperty,
key: string,
context: TSESLint.RuleContext<MessageIds, Options>
) {
if (
Expand All @@ -333,7 +343,7 @@ export function validateImplemenationNode(
context.report({
messageId: 'invalidImplementationPath',
data: {
key: (implementationNode.key as AST.JSONLiteral).value,
key,
},
node: implementationNode.value as any,
});
Expand All @@ -353,7 +363,7 @@ export function validateImplemenationNode(
context.report({
messageId: 'invalidImplementationPath',
data: {
key: (implementationNode.key as AST.JSONLiteral).value,
key,
},
node: implementationNode.value as any,
});
Expand All @@ -367,6 +377,7 @@ export function validateImplemenationNode(
node: implementationNode.value as any,
data: {
identifier,
key,
},
});
}
Expand All @@ -385,37 +396,60 @@ export function validatePackageGroup(
(x.key.value === 'nx-migrations' ||
x.key.value === 'ng-update' ||
x.key.value === 'migrations')
).value as AST.JSONObjectExpression;
)?.value as AST.JSONObjectExpression;

const packageGroupNode = migrationsNode.properties.find(
const packageGroupNode = migrationsNode?.properties.find(
(x) => x.key.type === 'JSONLiteral' && x.key.value === 'packageGroup'
);

if (packageGroupNode) {
// Package group is defined as an array
if (packageGroupNode.value.type === 'JSONArrayExpression') {
// Look at entries which are an object
const members = packageGroupNode.value.elements.filter(
(x) => x.type === 'JSONObjectExpression'
);
// validate that the version property exists and is valid
for (const member of members) {
const versionPropertyNode = (
member as AST.JSONObjectExpression
).properties.find(
(x) => x.key.type === 'JSONLiteral' && x.key.value === 'version'
);
if (versionPropertyNode && !versionPropertyNode.value) {
const packageNode = (
member as AST.JSONObjectExpression
).properties.find(
(x) => x.key.type === 'JSONLiteral' && x.key.value === 'package'
);
const key = (packageNode?.value as AST.JSONLiteral)?.value ?? 'unknown';

if (versionPropertyNode) {
if (!validateVersionJsonExpression(versionPropertyNode.value))
context.report({
messageId: 'invalidVersion',
data: { key },
node: versionPropertyNode.value as any,
});
} else {
context.report({
messageId: 'invalidVersion',
node: versionPropertyNode as any,
messageId: 'missingVersion',
data: { key },
node: member as any,
});
}
}
// Package group is defined as an object (Record<PackageName, Version>)
} else if (packageGroupNode.value.type === 'JSONObjectExpression') {
const properties = packageGroupNode.value.properties;
// For each property, ensure its value is a valid version
for (const propertyNode of properties) {
if (!validateVersionJsonExpression(propertyNode.value)) {
context.report({
messageId: 'invalidVersion',
node: propertyNode as any,
data: {
key: (propertyNode.key as AST.JSONLiteral).value,
},
node: propertyNode.value as any,
});
}
}
Expand All @@ -425,6 +459,7 @@ export function validatePackageGroup(

export function validateVersionJsonExpression(node: AST.JSONExpression) {
return (
node &&
node.type === 'JSONLiteral' &&
typeof node.value === 'string' &&
valid(node.value)
Expand Down
6 changes: 5 additions & 1 deletion packages/nx-plugin/src/generators/migration/migration.ts
View file
Expand Up @@ -11,7 +11,8 @@ import {
import type { Tree } from '@nrwl/devkit';
import type { Schema } from './schema';
import * as path from 'path';

import { addMigrationJsonChecks } from '../plugin-lint-checks/generator';
import type { Linter as EsLint } from 'eslint';
interface NormalizedSchema extends Schema {
projectRoot: string;
projectSourceRoot: string;
Expand Down Expand Up @@ -124,6 +125,9 @@ export async function migrationGenerator(host: Tree, schema: Schema) {
const options = normalizeOptions(host, schema);

addFiles(host, options);
if (!host.exists('migrations.json')) {
addMigrationJsonChecks(host, { projectName: schema.project });
}
updateMigrationsJson(host, options);
updateWorkspaceJson(host, options);
updateMigrationsJson(host, options);
Expand Down
2 changes: 1 addition & 1 deletion packages/nx-plugin/src/generators/plugin-lint-checks/generator.spec.ts
View file
Expand Up @@ -45,7 +45,7 @@ describe('plugin-lint generator', () => {
expect.objectContaining({
files: ['executors.json', 'package.json', 'generators.json'],
rules: {
'@nrwl/nx/nx-plugin-schema': 'error',
'@nrwl/nx/nx-plugin-checks': 'error',
},
})
);
Expand Down

0 comments on commit c3e5993

Please sign in to comment.