Skip to content
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

fix: lint does not work on new Otter webapp #1671

Merged
merged 1 commit into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/@o3r/core/schematics/ng-add/utils/linter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ export const shouldOtterLinterBeInstalled = async (context: SchematicContext): P
try {
require.resolve(`${linterPackageName}/package.json`);
if (context.interactive) {
useOtterLinter = await askConfirmation(`You already have eslint installed. Would you like to add otter config rules for eslint?
useOtterLinter = await askConfirmation(`You already have ESLint installed. Would you like to add otter config rules for ESLint?
Otherwise, you can add them later via this command: ng add @o3r/eslint-config-otter`, true);
}
} catch {
context.logger.info(`eslint package not installed. Skipping otter linter phase!
You can add otter linter config rules later to the project via this command: ng add @o3r/eslint-config-otter`);
context.logger.info(`ESLint package not installed. Skipping Otter linter phase!
You can add Otter linter config rules later to the project via this command: ng add @o3r/eslint-config-otter`);
}

return useOtterLinter;
Expand Down
3 changes: 2 additions & 1 deletion packages/@o3r/eslint-config-otter/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ module.exports = {
'sourceType': 'module',
'project': [
'tsconfig.builders.json',
'tsconfig.eslint.json'
'tsconfig.eslint.json',
'tsconfig.spec.json'
]
},
'extends': [
Expand Down
1 change: 1 addition & 0 deletions packages/@o3r/eslint-config-otter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"@o3r/build-helpers": "workspace:^",
"@o3r/eslint-plugin": "workspace:^",
"@o3r/schematics": "workspace:^",
"@o3r/test-helpers": "workspace:^",
"@schematics/angular": "~17.3.0",
"@stylistic/eslint-plugin-ts": "^1.5.4",
"@types/jest": "~29.5.2",
Expand Down
6 changes: 6 additions & 0 deletions packages/@o3r/eslint-config-otter/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
"jestConfig": "packages/@o3r/eslint-config-otter/jest.config.js"
}
},
"test-int": {
"executor": "@nx/jest:jest",
"options": {
"jestConfig": "packages/@o3r//eslint-config-otter/testing/jest.config.it.js"
}
},
"lint": {
"options": {
"eslintConfig": "packages/@o3r/eslint-config-otter/.eslintrc.js",
Expand Down
25 changes: 25 additions & 0 deletions packages/@o3r/eslint-config-otter/schematics/index.it.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Test environment exported by O3rEnvironment, must be first line of the file
* @jest-environment @o3r/test-helpers/jest-environment
* @jest-environment-o3r-app-folder test-app-eslint-config
*/
const o3rEnvironment = globalThis.o3rEnvironment;

import {
getDefaultExecSyncOptions,
packageManagerExec,
packageManagerInstall,
packageManagerRunOnProject
} from '@o3r/test-helpers';

describe('new otter application with eslint-config', () => {
test('should add eslint-config to existing application', () => {
const { workspacePath, projectName, isInWorkspace, o3rVersion } = o3rEnvironment.testEnvironment;
const execAppOptions = {...getDefaultExecSyncOptions(), cwd: workspacePath};
packageManagerExec({script: 'ng', args: ['add', `@o3r/eslint-config-otter@${o3rVersion}`, '--skip-confirmation', '--project-name', projectName]}, execAppOptions);

expect(() => packageManagerInstall(execAppOptions)).not.toThrow();
expect(() => packageManagerRunOnProject(projectName, isInWorkspace, {script: 'build'}, execAppOptions)).not.toThrow();
expect(() => packageManagerExec({script: 'ng', args: ['lint', projectName]}, execAppOptions)).not.toThrow('Invalid lint configuration. Nothing to lint.');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@ describe('Generate linter files', () => {

expect(tree.exists('.eslintignore')).toBe(true);
expect(tree.exists('.eslintrc.js')).toBe(true);
expect(tree.exists('tsconfig.eslint.json')).toBe(true);
});

it('should not overwrite eslintignore but extend @o3r/eslint-config-otter', async () => {
initialTree.create('.eslintignore', 'I am inevitable');
initialTree.create('.eslintrc.json', '{}');
const tree = await lastValueFrom(callRule(updateLinterConfigs({}, path.join(__dirname, '..')), initialTree, context));

expect(tree.readText('.eslintignore')).toBe('I am inevitable');
expect(tree.readJson('.eslintrc.json')).toEqual({'extends': ['@o3r/eslint-config-otter']});
});

it('should not overwrite linter files', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { apply, chain, MergeStrategy, mergeWith, renameTemplateFiles, Rule, SchematicContext, template, Tree, url } from '@angular-devkit/schematics';
import { apply, chain, MergeStrategy, mergeWith, move, renameTemplateFiles, Rule, SchematicContext, template, Tree, url } from '@angular-devkit/schematics';
import { posix } from 'node:path';

const tsEslintParserDep = '@typescript-eslint/parser';
/**
* Add or update the Linter configuration
* @param options @see RuleFactory.options
Expand All @@ -16,34 +16,77 @@ export function updateLinterConfigs(options: { projectName?: string | null | und
*/
const updateTslintExtend: Rule = async (tree: Tree, context: SchematicContext) => {
const eslintFilePath = '/.eslintrc.json';
const eslintExists = tree.exists(eslintFilePath);

if (eslintExists) {
if (tree.exists(eslintFilePath)) {
const eslintFile = tree.readJson(eslintFilePath) as { extends?: string | string[] };
eslintFile.extends = eslintFile.extends ? (eslintFile.extends instanceof Array ? eslintFile.extends : [eslintFile.extends]) : [];

if (eslintFile.extends.indexOf(tsEslintParserDep) === -1) {
eslintFile.extends.push(tsEslintParserDep);
const eslintConfigOtter = '@o3r/eslint-config-otter';
if (eslintFile.extends.indexOf(eslintConfigOtter) === -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if it extends a config that extends '@o3r/eslint-config-otter'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a corner case that isn't currently covered

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslintFile.extends.push(eslintConfigOtter);
}

tree.overwrite(eslintFilePath, JSON.stringify(eslintFile, null, 2));
} else {
const { getAllFilesInTree, getTemplateFolder } = await import('@o3r/schematics');
const eslintConfigFiles = getAllFilesInTree(tree, '/', ['**/.eslintrc.json'], false).filter((file) => /\.eslintrc/i.test(file));
const eslintConfigFiles = getAllFilesInTree(tree, '/', ['**/.eslintrc.js'], false).filter((file) => /\.eslintrc/i.test(file));
if (!eslintConfigFiles.length) {
return mergeWith(apply(url(getTemplateFolder(rootPath, __dirname)), [
return mergeWith(apply(url(getTemplateFolder(rootPath, __dirname, 'templates/workspace')), [
template({
dot: '.'
}),
renameTemplateFiles()
]), MergeStrategy.Overwrite);
} else {
context.logger.warn('An unsupported format EsLint configuration already exists, an automatic update cannot be applied.');
context.logger.warn(`You can manually extends "@o3r/eslint-config-otter" in your configuration ${eslintConfigFiles.map((f) => `"${f}"`).join(', ')}`);
context.logger.warn('An unsupported format ESLint configuration already exists, an automatic update cannot be applied.');
context.logger.warn(`You can manually extend "@o3r/eslint-config-otter" in your configuration ${eslintConfigFiles.map((f) => `"${f}"`).join(', ')}`);
return;
}
}
};

/**
* Create linter files for the project
* @param tree
* @param context
*/
const createProjectFiles: Rule = async (tree: Tree, context: SchematicContext) => {
if (!options.projectName) {
return;
}
const { getWorkspaceConfig } = await import('@o3r/schematics');
const workspace = getWorkspaceConfig(tree);
if (!workspace) {
return;
}
const workspaceProject = workspace.projects[options.projectName];

if (!workspaceProject) {
context.logger.warn('No project detected, linter task can not be added.');
return;
}

const projectRoot = workspaceProject.root;
const projectType = workspaceProject.projectType;
const eslintFilePath = posix.join(projectRoot, '/.eslintrc.js');

if (tree.exists(eslintFilePath)) {
context.logger.info(`${eslintFilePath} already exists.`);
return;
} else {
const { getTemplateFolder } = await import('@o3r/schematics');
const rootRelativePath = posix.relative(projectRoot, tree.root.path.replace(/^\//, './'));
return mergeWith(apply(url(getTemplateFolder(rootPath, __dirname, 'templates/project')), [
template({
dot: '.',
projectTsConfig: projectType === 'application' ? 'tsconfig.app' : 'tsconfig.lib',
rootRelativePath
}),
move(projectRoot),
renameTemplateFiles()
]), MergeStrategy.Overwrite);
}
};
/**
* Update angular.json file to use ESLint builder.
* @param tree
Expand All @@ -52,20 +95,23 @@ export function updateLinterConfigs(options: { projectName?: string | null | und
const editAngularJson: Rule = async (tree: Tree, context: SchematicContext) => {
const { getWorkspaceConfig } = await import('@o3r/schematics');
const workspace = getWorkspaceConfig(tree);
const workspaceProject = options.projectName ? workspace?.projects[options.projectName] : undefined;
if (!workspace) {
return;
}
const workspaceProject = options.projectName ? workspace.projects[options.projectName] : undefined;

if (!workspace || !workspaceProject) {
context.logger.warn('No project detected, linter task can not be added');
if (!workspaceProject) {
context.logger.warn('No project detected, linter task can not be added.');
return;
}

workspaceProject.architect ||= {};
workspaceProject.architect.lint ||= {
builder: '@angular-eslint/builder:lint',
options: {
eslintConfig: './.eslintrc.js',
eslintConfig: `${workspaceProject.root}/.eslintrc.js`,
lintFilePatterns: [
'src/**/*.ts'
`${workspaceProject.sourceRoot || posix.join(workspaceProject.root, 'src') }/**/*.ts`
]
}
};
Expand All @@ -76,6 +122,7 @@ export function updateLinterConfigs(options: { projectName?: string | null | und

return chain([
updateTslintExtend,
createProjectFiles,
editAngularJson
]);
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
/* eslint-disable @typescript-eslint/naming-convention */
/* eslint-disable quote-props */

module.exports = {
'root': true,
'parserOptions': {
'tsconfigRootDir': __dirname,
'project': [
'tsconfig.build.json',
'<%= projectTsConfig %>.json',
'tsconfig.spec.json',
'tsconfig.builders.json',
'tsconfig.eslint.json'
],
'sourceType': 'module'
},
'extends': [
'<%= eslintRcPath %>'
'<%= rootRelativePath %>/.eslintrc.js'
]
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extends": "./<%= projectTsConfig %>",
"include": [
".eslintrc.js"
]
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/node_modules
**/dist
**/dist-*
**/test
**/tmp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ module.exports = {
'parser': require.resolve('@typescript-eslint/parser'),
'parserOptions': {
'tsconfigRootDir': __dirname,
'project': [
'tsconfig.eslint.json'
],
'ecmaVersion': 12
},
'env': {
Expand Down
8 changes: 8 additions & 0 deletions packages/@o3r/eslint-config-otter/testing/jest.config.it.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const { join } = require('node:path');
const getJestConfig = require('../../../../jest.config.it').getJestConfig;

/** @type {import('ts-jest/dist/types').JestConfigWithTsJest} */
module.exports = {
...getJestConfig(join(__dirname, '..')),
displayName: require('../package.json').name
};
2 changes: 1 addition & 1 deletion packages/@o3r/eslint-plugin/tsconfig.spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
}
],
"include": [
"./src/**/*.spec.ts",
"**/*.spec.ts"
],
"exclude": []
}
2 changes: 1 addition & 1 deletion packages/@o3r/schematics/src/utility/monorepo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { getWorkspaceConfig } from './loaders';
/**
* Find the relative path to a configuration file at the monorepo root
* @param tree
* @param files List of files to look for, the first of the list will used
* @param files List of files to look for, the first of the list will be used
* @param originPath Path from where to calculate the relative path
* @returns
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/@o3r/workspace/schematics/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ function generateApplicationFn(options: NgGenerateApplicationSchema): Rule {
projectRoot,
style: Style.Scss}),
addProjectSpecificFiles(targetPath, rootDependencies),
updateProjectTsConfig(targetPath, 'tsconfig.app.json'),
updateProjectTsConfig(targetPath, 'tsconfig.app.json', {updateInputFiles: true}),
setupDependencies({
dependencies,
skipInstall: options.skipInstall,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,16 @@ export function ngGenerateModule(options: NgGenerateModuleSchema & { targetPath:
*/
const updateNgTemplate: Rule = (tree, context) => {
const o3rCorePackageJsonPath = path.resolve(__dirname, '..', '..', '..', 'package.json');
const o3rCorePackageJson: PackageJson & { generatorDependencies?: Record<string, string> } = JSON.parse(readFileSync(o3rCorePackageJsonPath)!.toString());
const o3rCorePackageJson: PackageJson & { generatorDependencies?: Record<string, string> } = JSON.parse(readFileSync(o3rCorePackageJsonPath).toString());
const otterVersion = o3rCorePackageJson.dependencies!['@o3r/schematics'];

const templateNg = apply(url('./templates/ng'), [
template({
...options,
runner: process.env.npm_execpath && /[\\/][^\\/]yarn[^\\/]js$/.test(process.env.npm_execpath) ? 'yarn run' : 'npm run',
tsconfigSpecPath: findConfigFileRelativePath(tree,
['tsconfig.test.json', 'tsconfig.spec.json', 'tsconfig.jest.json', 'tsconfig.jasmine.json', 'tsconfig.base.json', 'tsconfig.json'], options.targetPath),
tsconfigBasePath: findConfigFileRelativePath(tree, ['tsconfig.base.json', 'tsconfig.json'], options.targetPath),
tsconfigBuildPath: findConfigFileRelativePath(tree, ['tsconfig.build.json', 'tsconfig.base.json', 'tsconfig.json'], options.targetPath),
eslintRcPath: findConfigFileRelativePath(tree, ['.eslintrc.json', '.eslintrc.js'], options.targetPath)
tsconfigBuildPath: findConfigFileRelativePath(tree, ['tsconfig.build.json', 'tsconfig.base.json', 'tsconfig.json'], options.targetPath)
}),
renameTemplateFiles(),
move(options.targetPath)
Expand Down

This file was deleted.

6 changes: 3 additions & 3 deletions packages/@o3r/workspace/schematics/ng-add/helpers/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ export const shouldOtterLinterBeInstalled = async (context: SchematicContext): P
try {
require.resolve(`${linterPackageName}/package.json`);
if (context.interactive) {
useOtterLinter = await askConfirmation(`You already have eslint installed. Would you like to add otter config rules for eslint?
useOtterLinter = await askConfirmation(`You already have ESLint installed. Would you like to add Otter config rules for ESLint?
Otherwise, you can add them later via this command: ng add @o3r/eslint-config-otter`, true);
}
} catch {
context.logger.info(`eslint package not installed. Skipping otter linter phase!
You can add otter linter config rules later to the project via this command: ng add @o3r/eslint-config-otter`);
context.logger.info(`ESLint package not installed. Skipping otter linter phase!
You can add Otter linter config rules later to the project via this command: ng add @o3r/eslint-config-otter`);
}

return useOtterLinter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export function updateProjectTsConfig(targetPath: string, tsconfigName: string,

if (options?.updateInputFiles) {
tsconfig.config = Object.fromEntries(Object.entries(tsconfig.config).filter(([propName, _]) => propName !== 'files'));
tsconfig.config.exclude = ['**/*.spec.ts', '**/fixture'];
tsconfig.config.include = ['./src/**/*.ts'];
}
const baseTsConfig = findConfigFileRelativePath(tree, ['tsconfig.base.json', 'tsconfig.json'], targetPath);
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7492,6 +7492,7 @@ __metadata:
"@o3r/build-helpers": "workspace:^"
"@o3r/eslint-plugin": "workspace:^"
"@o3r/schematics": "workspace:^"
"@o3r/test-helpers": "workspace:^"
"@schematics/angular": "npm:~17.3.0"
"@stylistic/eslint-plugin-ts": "npm:^1.5.4"
"@types/jest": "npm:~29.5.2"
Expand Down
Loading