From 384c68d0408ca40b7d30c268847f8348546d5ecf Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Fri, 4 Oct 2024 15:45:15 +0200 Subject: [PATCH 1/8] Other: Treat type imports as `dependencies` instead of `devDependencies` --- .../lib/checkdependencies.js | 123 ++++++------------ 1 file changed, 39 insertions(+), 84 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index abe80870c..b3f64d5b9 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -82,7 +82,7 @@ async function checkDependenciesInPackage( packagePath, options ) { const result = await depCheck( packageAbsolutePath, depCheckOptions ); - const missingPackages = await groupMissingPackages( result.missing, packageJson.name ); + const missingPackages = groupMissingPackages( result.missing, packageJson.name ); const misplacedOptions = { dependencies: packageJson.dependencies, @@ -132,7 +132,7 @@ async function checkDependenciesInPackage( packagePath, options ) { // Misplaced `dependencies` or `devDependencies`. // Checks whether any package, which is already listed in the `dependencies` or `devDependencies`, // should belong to that list. - ( await findMisplacedDependencies( misplacedOptions ) ) + findMisplacedDependencies( misplacedOptions ) .reduce( ( result, group ) => { return result + '\n' + group.description + '\n' + @@ -197,7 +197,7 @@ function getInvalidItselfImports( repositoryPath ) { * @param {string} currentPackage Name of current package. * @returns {Promise.>>} */ -async function groupMissingPackages( missingPackages, currentPackage ) { +function groupMissingPackages( missingPackages, currentPackage ) { delete missingPackages[ currentPackage ]; const dependencies = []; @@ -206,10 +206,10 @@ async function groupMissingPackages( missingPackages, currentPackage ) { for ( const packageName of Object.keys( missingPackages ) ) { const absolutePaths = missingPackages[ packageName ]; - if ( await isDevDependency( packageName, absolutePaths ) ) { - devDependencies.push( packageName ); - } else { + if ( isProductionDependency( packageName, absolutePaths ) ) { dependencies.push( packageName ); + } else { + devDependencies.push( packageName ); } } @@ -322,10 +322,10 @@ function findDuplicatedDependencies( dependencies, devDependencies ) { * @param {object|undefined} options.devDependencies Defined development dependencies from package.json. * @param {object} options.dependenciesToCheck All dependencies that have been found and files where they are used. * @param {Array.} options.dependenciesToIgnore An array of package names that should not be checked. - * @returns {Promise.>} Misplaced packages. Each array item is an object containing + * @returns {} Misplaced packages. Each array item is an object containing * the `description` string and `packageNames` array of strings. */ -async function findMisplacedDependencies( options ) { +function findMisplacedDependencies( options ) { const { dependencies, devDependencies, dependenciesToCheck, dependenciesToIgnore } = options; const deps = Object.keys( dependencies || {} ); const devDeps = Object.keys( devDependencies || {} ); @@ -346,9 +346,9 @@ async function findMisplacedDependencies( options ) { continue; } - const isDevDep = await isDevDependency( packageName, absolutePaths ); - const isMissingInDependencies = !isDevDep && !deps.includes( packageName ) && devDeps.includes( packageName ); - const isMissingInDevDependencies = isDevDep && deps.includes( packageName ) && !devDeps.includes( packageName ); + const isProdDep = isProductionDependency( packageName, absolutePaths ); + const isMissingInDependencies = isProdDep && !deps.includes( packageName ) && devDeps.includes( packageName ); + const isMissingInDevDependencies = !isProdDep && deps.includes( packageName ) && !devDeps.includes( packageName ); if ( isMissingInDependencies ) { misplacedPackages.missingInDependencies.packageNames.add( packageName ); @@ -369,89 +369,44 @@ async function findMisplacedDependencies( options ) { } /** - * Checks if a given package is a development-only dependency. Package is considered a dev dependency - * if it is used only in files that are not used in the final build, such as tests, demos or typings. - * - * @param {string} packageName - * @param {Array.} absolutePaths Files where a given package has been imported. - * @returns {Promise.} + * These folders contain code that will be shipped to npm and run in the final projects. + * This means that all dependencies used in these folders are production dependencies. */ -async function isDevDependency( packageName, absolutePaths ) { - if ( packageName.startsWith( '@types/' ) ) { - return true; - } - +const foldersContainingProductionCode = [ /** - * These folders contain code that will be shipped to npm and run in the final projects. - * This means that all dependencies used in these folders are production dependencies. + * These folders contain the source code of the packages. */ - const foldersContainingProductionCode = [ - /** - * These folders contain the source code of the packages. - */ - /[/\\]bin[/\\]/, - /[/\\]src[/\\]/, - /[/\\]lib[/\\]/, - /[/\\]theme[/\\]/, - - /** - * This folder contains the compiled code of the packages. Most of this code is the same - * as the source, but during the build process some of the imports are replaced with those - * compatible with the "new installation methods", which may use different dependencies. - * - * For example, the `ckeditor5/src/core.js` import is replaced with `@ckeditor/ckeditor5-core/dist/index.js`. - * ^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^ - */ - /[/\\]dist[/\\]/ - ]; - - for ( const absolutePath of absolutePaths ) { - if ( !foldersContainingProductionCode.some( folder => absolutePath.match( folder ) ) ) { - continue; - } - - if ( absolutePath.endsWith( '.ts' ) ) { - // Verify kind of imports in TypeScript file. - const importKinds = await getImportAndExportKinds( packageName, absolutePath ); + /[/\\]bin[/\\]/, + /[/\\]src[/\\]/, + /[/\\]lib[/\\]/, + /[/\\]theme[/\\]/, - // There is any non type kind of import from that package so not a dev dependency. - if ( importKinds.some( importKind => importKind != 'type' ) ) { - return false; - } - } else { - // Import from some other file from src/ or theme/ - package is not dev dependency. - return false; - } - } - - // There were no value imports, so it is a dev dependency. - return true; -} + /** + * This folder contains the compiled code of the packages. Most of this code is the same + * as the source, but during the build process some of the imports are replaced with those + * compatible with the "new installation methods", which may use different dependencies. + * + * For example, the `ckeditor5/src/core.js` import is replaced with `@ckeditor/ckeditor5-core/dist/index.js`. + * ^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^ + */ + /[/\\]dist[/\\]/ +]; /** - * Parses TS file from `absolutePath` and returns a list of import and export types from `packageName`. + * Checks if a given package is a production dependency, i.e., it's used in build files or their typings. * - * @param {string} packageName - * @param {string} absolutePath File where a given package has been imported. - * @returns {Promise.>} Array of import kinds. + * @param {string} packageName Name of the package. + * @param {Array.} paths Files where a given package has been imported. + * @returns {boolean} */ -async function getImportAndExportKinds( packageName, absolutePath ) { - const astContent = await depCheck.parser.typescript( absolutePath ); - - if ( !astContent || !astContent.program || !astContent.program.body ) { - return []; +function isProductionDependency( packageName, paths ) { + if ( packageName.startsWith( '@types/' ) ) { + return false; } - const types = [ - 'ImportDeclaration', - 'ExportAllDeclaration', - 'ExportNamedDeclaration' - ]; - - return astContent.program.body - .filter( node => types.includes( node.type ) ) - .filter( node => node.source?.value?.startsWith( packageName ) ) - .map( node => node.importKind || node.exportKind ); + return paths.some( + path => foldersContainingProductionCode.some( folder => path.match( folder ) ) + ); } /** From bb7578160320a33e549e4d359176098ea45dae57 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Thu, 31 Oct 2024 15:08:22 +0100 Subject: [PATCH 2/8] Use oxc-parser for ES modules for better performance. --- .../lib/checkdependencies.js | 51 ++++++++++++++++--- .../package.json | 3 ++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index b3f64d5b9..9d7962ac6 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -5,9 +5,10 @@ import fs from 'fs-extra'; import upath from 'upath'; -import { globSync } from 'glob'; -import depCheck from 'depcheck'; import chalk from 'chalk'; +import oxc from 'oxc-parser'; +import depCheck from 'depcheck'; +import { globSync } from 'glob'; /** * Checks dependencies sequentially in all provided packages. @@ -62,10 +63,10 @@ async function checkDependenciesInPackage( packagePath, options ) { parsers: { '**/*.css': filePath => parsePostCSS( filePath, onMissingCSSFile ), '**/*.cjs': depCheck.parser.es6, - '**/*.mjs': depCheck.parser.es6, - '**/*.js': depCheck.parser.es6, - '**/*.jsx': depCheck.parser.jsx, - '**/*.ts': depCheck.parser.typescript, + '**/*.mjs': filePath => parseModule( filePath, depCheck.parser.es6 ), + '**/*.js': filePath => parseModule( filePath, depCheck.parser.es6 ), + '**/*.jsx': filePath => parseModule( filePath, depCheck.parser.jsx ), + '**/*.ts': filePath => parseModule( filePath, depCheck.parser.typescript ), '**/*.vue': depCheck.parser.vue }, ignorePatterns: [ 'docs', 'build', 'dist/browser' ], @@ -81,7 +82,6 @@ async function checkDependenciesInPackage( packagePath, options ) { } const result = await depCheck( packageAbsolutePath, depCheckOptions ); - const missingPackages = groupMissingPackages( result.missing, packageJson.name ); const misplacedOptions = { @@ -114,6 +114,7 @@ async function checkDependenciesInPackage( packagePath, options ) { // Unused devDependencies. result.devDependencies + .filter( entry => !entry.startsWith( '@types/' ) ) .map( entry => '- ' + entry ) .join( '\n' ), @@ -284,6 +285,42 @@ function parsePostCSS( filePath, onMissingCSSFile ) { return [ ...usedPackages ].sort(); } +/** + * Returns a list of external dependencies used in a given JavaScript module. + * The `fallbackParser` is used if file is not an ECMAScript module, but a CommonJS module. + * + * @param {string} filePath An absolute path to the checking file. + * @param {function} fallbackParser Fallback parser used when the file is not an ECMAScript module. + * @returns {Array.} + */ +async function parseModule( filePath, fallbackParser ) { + const fileContent = await fs.readFile( filePath, 'utf-8' ); + const result = await oxc.moduleLexerAsync( fileContent, { sourceFilename: filePath } ); + + if ( !result.hasModuleSyntax || result.imports.some( imp => !imp.n ) ) { + return fallbackParser( filePath ); + } + + return result.imports + // Filter out relative and absolute imports. + .filter( imp => !imp.n.startsWith( '.' ) && !imp.n.startsWith( '/' ) ) + // Get package names. + .map( imp => { + const segments = imp.n.split( '/' ); + + // Scoped package name. + if ( segments[ 0 ].startsWith( '@' ) ) { + return segments.slice( 0, 2 ).join( '/' ); + } + + // Non-scoped package name. + return segments[ 0 ]; + } ) + // Remove duplicates. + .filter( ( item, index, array ) => array.indexOf( item ) === index ) + .sort(); +} + /** * Checks whether packages specified as `devDependencies` are not duplicated with items defined as `dependencies`. * diff --git a/packages/ckeditor5-dev-dependency-checker/package.json b/packages/ckeditor5-dev-dependency-checker/package.json index 309984ef9..314577c45 100644 --- a/packages/ckeditor5-dev-dependency-checker/package.json +++ b/packages/ckeditor5-dev-dependency-checker/package.json @@ -32,5 +32,8 @@ "glob": "^10.0.0", "minimist": "^1.2.8", "upath": "^2.0.1" + }, + "devDependencies": { + "oxc-parser": "^0.34.0" } } From c7a5e5edf925c6d87e516f5169a247a61df12e78 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Jan 2025 11:48:49 +0100 Subject: [PATCH 3/8] Update oxc-parser --- .../lib/checkdependencies.js | 8 ++++---- packages/ckeditor5-dev-dependency-checker/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index 9d7962ac6..d1425c1b0 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -291,17 +291,17 @@ function parsePostCSS( filePath, onMissingCSSFile ) { * * @param {string} filePath An absolute path to the checking file. * @param {function} fallbackParser Fallback parser used when the file is not an ECMAScript module. - * @returns {Array.} + * @returns {Promise.>} */ async function parseModule( filePath, fallbackParser ) { const fileContent = await fs.readFile( filePath, 'utf-8' ); - const result = await oxc.moduleLexerAsync( fileContent, { sourceFilename: filePath } ); + const result = oxc.parseSync( filePath, fileContent ); - if ( !result.hasModuleSyntax || result.imports.some( imp => !imp.n ) ) { + if ( !result.module.hasModuleSyntax || result.module.staticImports.some( imp => !imp.n ) ) { return fallbackParser( filePath ); } - return result.imports + return result.module.staticImports // Filter out relative and absolute imports. .filter( imp => !imp.n.startsWith( '.' ) && !imp.n.startsWith( '/' ) ) // Get package names. diff --git a/packages/ckeditor5-dev-dependency-checker/package.json b/packages/ckeditor5-dev-dependency-checker/package.json index 314577c45..3c8b90681 100644 --- a/packages/ckeditor5-dev-dependency-checker/package.json +++ b/packages/ckeditor5-dev-dependency-checker/package.json @@ -34,6 +34,6 @@ "upath": "^2.0.1" }, "devDependencies": { - "oxc-parser": "^0.34.0" + "oxc-parser": "^0.44.0" } } From 27bfe76d94cecdc5db91e3dd9c7a138a310829da Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Jan 2025 14:06:43 +0100 Subject: [PATCH 4/8] Update `eslint-config-ckeditor5` --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 71ac15c69..f1b56f1e7 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "@listr2/prompt-adapter-inquirer": "^2.0.16", "@octokit/rest": "^21.0.0", "eslint": "^8.21.0", - "eslint-config-ckeditor5": "^8.0.0", + "eslint-config-ckeditor5": "^9.0.0", "fs-extra": "^11.0.0", "glob": "^10.0.0", "husky": "^8.0.2", From 9a5b6201de0556da8907a0fdbabc269301de8631 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Jan 2025 15:27:58 +0100 Subject: [PATCH 5/8] Update `parseModule` implementation --- .../lib/checkdependencies.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index d1425c1b0..7030fc22d 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -295,18 +295,23 @@ function parsePostCSS( filePath, onMissingCSSFile ) { */ async function parseModule( filePath, fallbackParser ) { const fileContent = await fs.readFile( filePath, 'utf-8' ); - const result = oxc.parseSync( filePath, fileContent ); + const { hasModuleSyntax, staticImports, staticExports } = oxc.parseSync( filePath, fileContent ).module; - if ( !result.module.hasModuleSyntax || result.module.staticImports.some( imp => !imp.n ) ) { + if ( !hasModuleSyntax ) { return fallbackParser( filePath ); } - return result.module.staticImports + return [ + ...staticImports, + ...staticExports.map( exp => exp.entries ) + ] + .flat() + .map( statement => statement.moduleRequest?.value ) // Filter out relative and absolute imports. - .filter( imp => !imp.n.startsWith( '.' ) && !imp.n.startsWith( '/' ) ) + .filter( path => path && !path.startsWith( '.' ) && !path.startsWith( '/' ) ) // Get package names. - .map( imp => { - const segments = imp.n.split( '/' ); + .map( path => { + const segments = path.split( '/' ); // Scoped package name. if ( segments[ 0 ].startsWith( '@' ) ) { From fa4510ad60d1f802494732722dc01f62875cd0cf Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Jan 2025 15:29:12 +0100 Subject: [PATCH 6/8] Update --- .../ckeditor5-dev-dependency-checker/lib/checkdependencies.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index 7030fc22d..c88bb6aae 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -303,9 +303,8 @@ async function parseModule( filePath, fallbackParser ) { return [ ...staticImports, - ...staticExports.map( exp => exp.entries ) + ...staticExports.flatMap( exp => exp.entries ) ] - .flat() .map( statement => statement.moduleRequest?.value ) // Filter out relative and absolute imports. .filter( path => path && !path.startsWith( '.' ) && !path.startsWith( '/' ) ) From 1381a8bfcd7e953233fa9a19523655be9ac1f7c4 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Jan 2025 15:34:46 +0100 Subject: [PATCH 7/8] Remove `oxc-parser` --- .../lib/checkdependencies.js | 49 ++----------------- .../package.json | 3 -- 2 files changed, 4 insertions(+), 48 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index c88bb6aae..7488f8a50 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -6,7 +6,6 @@ import fs from 'fs-extra'; import upath from 'upath'; import chalk from 'chalk'; -import oxc from 'oxc-parser'; import depCheck from 'depcheck'; import { globSync } from 'glob'; @@ -63,10 +62,10 @@ async function checkDependenciesInPackage( packagePath, options ) { parsers: { '**/*.css': filePath => parsePostCSS( filePath, onMissingCSSFile ), '**/*.cjs': depCheck.parser.es6, - '**/*.mjs': filePath => parseModule( filePath, depCheck.parser.es6 ), - '**/*.js': filePath => parseModule( filePath, depCheck.parser.es6 ), - '**/*.jsx': filePath => parseModule( filePath, depCheck.parser.jsx ), - '**/*.ts': filePath => parseModule( filePath, depCheck.parser.typescript ), + '**/*.mjs': depCheck.parser.es6, + '**/*.js': depCheck.parser.es6, + '**/*.jsx': depCheck.parser.jsx, + '**/*.ts': depCheck.parser.typescript, '**/*.vue': depCheck.parser.vue }, ignorePatterns: [ 'docs', 'build', 'dist/browser' ], @@ -285,46 +284,6 @@ function parsePostCSS( filePath, onMissingCSSFile ) { return [ ...usedPackages ].sort(); } -/** - * Returns a list of external dependencies used in a given JavaScript module. - * The `fallbackParser` is used if file is not an ECMAScript module, but a CommonJS module. - * - * @param {string} filePath An absolute path to the checking file. - * @param {function} fallbackParser Fallback parser used when the file is not an ECMAScript module. - * @returns {Promise.>} - */ -async function parseModule( filePath, fallbackParser ) { - const fileContent = await fs.readFile( filePath, 'utf-8' ); - const { hasModuleSyntax, staticImports, staticExports } = oxc.parseSync( filePath, fileContent ).module; - - if ( !hasModuleSyntax ) { - return fallbackParser( filePath ); - } - - return [ - ...staticImports, - ...staticExports.flatMap( exp => exp.entries ) - ] - .map( statement => statement.moduleRequest?.value ) - // Filter out relative and absolute imports. - .filter( path => path && !path.startsWith( '.' ) && !path.startsWith( '/' ) ) - // Get package names. - .map( path => { - const segments = path.split( '/' ); - - // Scoped package name. - if ( segments[ 0 ].startsWith( '@' ) ) { - return segments.slice( 0, 2 ).join( '/' ); - } - - // Non-scoped package name. - return segments[ 0 ]; - } ) - // Remove duplicates. - .filter( ( item, index, array ) => array.indexOf( item ) === index ) - .sort(); -} - /** * Checks whether packages specified as `devDependencies` are not duplicated with items defined as `dependencies`. * diff --git a/packages/ckeditor5-dev-dependency-checker/package.json b/packages/ckeditor5-dev-dependency-checker/package.json index 5b38b93bd..dc70f4206 100644 --- a/packages/ckeditor5-dev-dependency-checker/package.json +++ b/packages/ckeditor5-dev-dependency-checker/package.json @@ -32,8 +32,5 @@ "glob": "^10.0.0", "minimist": "^1.2.8", "upath": "^2.0.1" - }, - "devDependencies": { - "oxc-parser": "^0.44.0" } } From 6038e6ede1394a57fa9a4fa80a451a45f8f8a670 Mon Sep 17 00:00:00 2001 From: Filip Sobol Date: Wed, 8 Jan 2025 15:36:23 +0100 Subject: [PATCH 8/8] Remove unnecessary changes --- .../lib/checkdependencies.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js index 7488f8a50..e3632f9cf 100644 --- a/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js +++ b/packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js @@ -5,9 +5,9 @@ import fs from 'fs-extra'; import upath from 'upath'; -import chalk from 'chalk'; -import depCheck from 'depcheck'; import { globSync } from 'glob'; +import depCheck from 'depcheck'; +import chalk from 'chalk'; /** * Checks dependencies sequentially in all provided packages. @@ -113,7 +113,6 @@ async function checkDependenciesInPackage( packagePath, options ) { // Unused devDependencies. result.devDependencies - .filter( entry => !entry.startsWith( '@types/' ) ) .map( entry => '- ' + entry ) .join( '\n' ),