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.

Sign up for GitHub

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

Treat type dependencies as production dependencies #1054

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Treat type dependencies as production dependencies #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
384c68d
Other: Treat type imports as `dependencies` instead of `devDependencies`
filipsobol Oct 4, 2024
ae5917f
Merge remote-tracking branch 'origin/master' into types-as-dependencies
filipsobol Oct 31, 2024
bb75781
Use oxc-parser for ES modules for better performance.
filipsobol Oct 31, 2024
9ed18cd
Merge remote-tracking branch 'origin/master' into use-oxc-parser-in-d…
filipsobol Dec 2, 2024
c7a5e5e
Update oxc-parser
filipsobol Jan 8, 2025
0a4887f
Merge remote-tracking branch 'origin/master' into use-oxc-parser-in-d…
filipsobol Jan 8, 2025
27bfe76
Update `eslint-config-ckeditor5`
filipsobol Jan 8, 2025
9a5b620
Update `parseModule` implementation
filipsobol Jan 8, 2025
fa4510a
Update
filipsobol Jan 8, 2025
1381a8b
Remove `oxc-parser`
filipsobol Jan 8, 2025
6038e6e
Remove unnecessary changes
filipsobol Jan 8, 2025
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
124 changes: 39 additions & 85 deletions packages/ckeditor5-dev-dependency-checker/lib/checkdependencies.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,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,
Expand Down Expand Up @@ -132,7 +131,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' +
Expand Down Expand Up @@ -197,7 +196,7 @@ function getInvalidItselfImports( repositoryPath ) {
* @param {string} currentPackage Name of current package.
* @returns {Promise.<Object.<string, Array.<string>>>}
*/
async function groupMissingPackages( missingPackages, currentPackage ) {
function groupMissingPackages( missingPackages, currentPackage ) {
delete missingPackages[ currentPackage ];

const dependencies = [];
Expand All @@ -206,10 +205,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 );
}
}

Expand Down Expand Up @@ -322,10 +321,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.<string>} options.dependenciesToIgnore An array of package names that should not be checked.
* @returns {Promise.<Array.<object>>} Misplaced packages. Each array item is an object containing
* @returns {<Array.<object>} 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 || {} );
Expand All @@ -346,9 +345,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 );
Expand All @@ -369,89 +368,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.<string>} absolutePaths Files where a given package has been imported.
* @returns {Promise.<boolean>}
* 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;
}
/[/\\]bin[/\\]/,
/[/\\]src[/\\]/,
/[/\\]lib[/\\]/,
/[/\\]theme[/\\]/,

if ( absolutePath.endsWith( '.ts' ) ) {
// Verify kind of imports in TypeScript file.
const importKinds = await getImportAndExportKinds( packageName, absolutePath );

// 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.<string>>} Array of import kinds.
* @param {string} packageName Name of the package.
* @param {Array.<string>} 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 ) )
);
}

/**
Expand Down
Loading