Skip to content

Commit

Permalink
Merge pull request #33 from ckeditor/update-allow-imports-only-from-m…
Browse files Browse the repository at this point in the history
…ain-package-entry-point-rule

Fix (eslint-plugin-ckeditor5-rules): Fix `ckeditor5-rules/require-file-extensions-in-imports` rule for packages with `exports` that do not automatically add file extension.

Feature (eslint-plugin-ckeditor5-rules): Add the `ckeditor5-rules/no-legacy-imports` rule to detect and fix imports used in the old installation methods.

Feature (eslint-plugin-ckeditor5-rules): The `ckeditor5-rules/allow-imports-only-from-main-package-entry-point` rule should disallow importing from any path other than the package main entry point. Closes ckeditor/ckeditor5#16559

MINOR BREAKING CHANGE (eslint-plugin-ckeditor5-rules): Removed the `ckeditor5-rules/no-cross-package-svg-imports` rule, as it does the same thing as the updated `allow-imports-only-from-main-package-entry-point` rule.
  • Loading branch information
pomek committed Jun 18, 2024
2 parents 42d83b4 + 6fab18a commit 848ae15
Show file tree
Hide file tree
Showing 8 changed files with 275 additions and 128 deletions.
2 changes: 1 addition & 1 deletion packages/eslint-plugin-ckeditor5-rules/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

module.exports = {
rules: {
'no-legacy-imports': require( './rules/no-legacy-imports' ),
'no-relative-imports': require( './rules/no-relative-imports' ),
'ckeditor-error-message': require( './rules/ckeditor-error-message' ),
'ckeditor-imports': require( './rules/ckeditor-imports' ),
'no-cross-package-svg-imports': require( './rules/no-cross-package-svg-imports' ),
'no-cross-package-imports': require( './rules/no-cross-package-imports' ),
'no-scoped-imports-within-package': require( './rules/no-scoped-imports-within-package' ),
'license-header': require( './rules/license-header' ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,21 @@ module.exports = {
return;
}

if ( !node.source.value.startsWith( '@ckeditor/' ) ) {
// Ignore packages whose names don't start with '@ckeditor/'.
const path = node.source.value;

if ( !path.startsWith( '@ckeditor/' ) ) {
// Ignore imports that do not start with '@ckeditor/'.
return;
}

if ( !node.source.value.includes( '/src/' ) ) {
// Ignore imports that don't include 'src'.
if ( path.split( '/' ).length === 2 ) {
// Ignore imports from the main package entry point.
return;
}

context.report( {
node,
// eslint-disable-next-line max-len
message: 'Importing from "@ckeditor/*" packages is only allowed from the main package entry point, not from their "/src" folder.'
message: 'Importing from "@ckeditor/*" packages is only allowed from the main package entry point.'
} );
}
};
Expand Down

This file was deleted.

143 changes: 143 additions & 0 deletions packages/eslint-plugin-ckeditor5-rules/lib/rules/no-legacy-imports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/**
* @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md.
*/

'use strict';

// Allowed packages to be checked.
const corePackageName = 'ckeditor5';
const premiumPackageName = 'ckeditor5-premium-features';

// Default packages to check.
const defaultPackages = [
corePackageName,
premiumPackageName
];

// Package dependency cache to avoid excessive file reads.
const cache = new Map();

/**
* Returns an array of dependencies of a given package.
*
* @param {string} packageName
* @returns {Array<string>}
*/
function getDependencies( packageName ) {
if ( cache.has( packageName ) ) {
return cache.get( packageName );
}

try {
const packageJsonPath = require.resolve(
`${ packageName }/package.json`,
{ paths: [ process.cwd() ] }
);

const packageJson = require( packageJsonPath );
const dependencies = Object.keys( packageJson.dependencies || {} );

cache.set( packageName, dependencies );

return dependencies;
} catch {
cache.set( packageName, [] );

return [];
}
}

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'Disallow legacy imports from CKEditor5 packages.',
category: 'CKEditor5',
// eslint-disable-next-line max-len
url: 'https://ckeditor.com/docs/ckeditor5/latest/framework/contributing/code-style.html#no-legacy-imports'
},
fixable: 'code',
messages: {
legacyImport: 'Import must be done from the "{{ packageName }}" package'
},
schema: [
{
type: 'object',
properties: {
packages: {
type: 'array',
items: [
// Allow only core and premium packages to be checked.
{
anyOf: [
{
enum: [ corePackageName, premiumPackageName ]
}
]
}
]
}
}
}
]
},
create( context ) {
// Get the packages to check from the rule options.
const { packages = defaultPackages } = context.options[ 0 ] || {};

/**
* Checks if the given path is an invalid core package import.
*
* @param {string} path
* @returns boolean
*/
function isInvalidCoreImport( path ) {
return packages.includes( corePackageName ) &&
getDependencies( corePackageName ).includes( path ) ||
path.startsWith( 'ckeditor5/src/' );
}

/**
* Checks if the given path is an invalid premium package import.
*
* @param {string} path
* @returns boolean
*/
function isInvalidPremiumImport( path ) {
return packages.includes( premiumPackageName ) &&
getDependencies( premiumPackageName ).includes( path ) ||
path.startsWith( 'ckeditor5-collaboration/src/' );
}

function report( node, packageName ) {
// Get the original quoting style from the source code (" or ').
const quoteStyle = node.source.raw[ 0 ];

return context.report( {
node,
messageId: 'legacyImport',
data: {
packageName
},
fix( fixer ) {
return fixer.replaceText( node.source, quoteStyle + packageName + quoteStyle );
}
} );
}

return {
ImportDeclaration( node ) {
const path = node.source.value;

if ( isInvalidCoreImport( path ) ) {
return report( node, corePackageName );
}

if ( isInvalidPremiumImport( path ) ) {
return report( node, premiumPackageName );
}
}
};
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

'use strict';

const { normalize, extname, dirname, sep, parse, join } = require( 'upath' );
const { normalize, extname, dirname, sep, parse } = require( 'upath' );
const { isBuiltin } = require( 'module' );
const { exports: resolveExports } = require( 'resolve.exports' );
const enhancedResolve = require( 'enhanced-resolve' );
Expand Down Expand Up @@ -198,19 +198,5 @@ function isExternalPackage( url ) {
return false;
}

try {
// Resolve "exports" in the package.json file.
for ( const path of resolveExports( packageJson, url ) ) {
require.resolve(
join( dirname( packageJsonPath ), path )
);

return true;
}
} catch {
return false;
}

// URL is a valid npm package, but the path after the package name is not valid, likely missing a file extension.
return false;
return resolveExports( packageJson, url ).every( extname );
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,30 @@ ruleTester.run(
'import { Table } from "@ckeditor/ckeditor5-table";'
],
invalid: [
// Do not allow importing from the `src` folder.
{
code: 'import Table from "@ckeditor/ckeditor5-table/src/table";',
errors: [
{
// eslint-disable-next-line max-len
message: 'Importing from "@ckeditor/*" packages is only allowed from the main package entry point, not from their "/src" folder.'
message: 'Importing from "@ckeditor/*" packages is only allowed from the main package entry point.'
}
]
},
// Do not allow importing icons from the `theme` folder.
{
code: 'import icon from "@ckeditor/ckeditor5-table/theme/icons/icon.svg";',
errors: [
{
message: 'Importing from "@ckeditor/*" packages is only allowed from the main package entry point.'
}
]
},
// Do not allow importing style sheets from the `theme` folder.
{
code: 'import styles from "@ckeditor/ckeditor5-table/theme/styles.css";',
errors: [
{
message: 'Importing from "@ckeditor/*" packages is only allowed from the main package entry point.'
}
]
}
Expand Down

This file was deleted.

Loading

0 comments on commit 848ae15

Please sign in to comment.