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

Chore/8201 deprecate json tokens #8246

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions packages/eslint-plugin/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package.tgz
1 change: 1 addition & 0 deletions packages/eslint-plugin/.npmignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
rules/*.test.js
package.tgz
28 changes: 23 additions & 5 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ This package contains an eslint plugin that enforces some default rules for usin

## Setup

1. install `@elastic/eslint-plugin-eui` as a dev dependency
2. extend `plugin:@elastic/eui/recommended` in your eslint config
1. Install `@elastic/eslint-plugin-eui` as a dev dependency.
2. Extend `plugin:@elastic/eui/recommended` in your ESLint config.

## Rules

Expand All @@ -15,11 +15,29 @@ This package contains an eslint plugin that enforces some default rules for usin

In some cases it makes sense to disable this rule locally, such as when <kbd>cmd</kbd>+click should open the link in a new tab, but a standard click should use the `history.pushState()` API to change the URL without triggering a full page load.

### `@elastic/eui/no-restricted-eui-imports`

At times, we deprecate features that may need more highlighting and/or that are not possible to annotate with JSDoc `@deprecated`, e.g. JSON token imports: `@elastic/eui/dist/eui_theme_*.json` (for context: https://github.com/elastic/kibana/issues/199715#json-tokens).

We don't use `no-restricted-imports` because ESLint doesn't allow multiple error levels at once and it may conflict with the consumer's existing ESLint configuration for that rule. We need to assure that our rule will produce a warning (as a recommendation).

All deprecations still must follow our [deprecation process](../../wiki/eui-team-processes/deprecations.md).

## Testing

### Against an existing package
weronikaolejniczak marked this conversation as resolved.
Show resolved Hide resolved

To test the local changes to the plugin, you must:

1. Run `yarn pack` in the directory.
2. In your project's `package.json`, point `@elastic/eslint-plugin-eui` to `file:/path/to/package.tgz`.
3. Install dependencies: `yarn`.

## Publishing

This package is published separately from the rest of EUI, as required by eslint. The code is not transpiled, so make sure to use `require()` statements rather than `import`, and once the code is updated run:

1. `npm version patch|minor|major`
2. commit version bump
3. `npm publish` in this directory
4. push the version bump upstream
2. Commit version bump.
3. `npm publish` in this directory.
4. Push the version bump upstream.
2 changes: 2 additions & 0 deletions packages/eslint-plugin/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
module.exports = {
rules: {
'href-or-on-click': require('./rules/href_or_on_click'),
'no-restricted-eui-imports': require('./rules/no_restricted_eui_imports'),
},
configs: {
recommended: {
plugins: ['@elastic/eslint-plugin-eui'],
rules: {
'@elastic/eui/href-or-on-click': 'warn',
'@elastic/eui/no-restricted-eui-imports': 'warn',
},
},
},
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/jest.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
6 changes: 5 additions & 1 deletion packages/eslint-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
"devDependencies": {
"@babel/core": "^7.6.0",
"babel-eslint": "^10.0.3",
"eslint": "^6.4.0"
"eslint": "^6.4.0",
"jest": "^29.7.0"
},
"scripts": {
"test": "jest"
}
}
8 changes: 5 additions & 3 deletions packages/eslint-plugin/rules/href_or_on_click.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ ruleTester.run('@elastic/eui/href-or-on-click', rule, {

errors: [
{
message: '<EuiButton> accepts either `href` or `onClick`, not both.',
message:
'<EuiButton> supplied with both `href` and `onClick`; is this intentional? (Valid use cases include programmatic navigation via `onClick` while preserving \"Open in new tab\" style functionality via `href`.)',
},
],
},
Expand All @@ -74,7 +75,7 @@ ruleTester.run('@elastic/eui/href-or-on-click', rule, {
errors: [
{
message:
'<EuiButtonEmpty> accepts either `href` or `onClick`, not both.',
'<EuiButtonEmpty> supplied with both `href` and `onClick`; is this intentional? (Valid use cases include programmatic navigation via `onClick` while preserving "Open in new tab" style functionality via `href`.)',
},
],
},
Expand All @@ -87,7 +88,8 @@ ruleTester.run('@elastic/eui/href-or-on-click', rule, {

errors: [
{
message: '<EuiLink> accepts either `href` or `onClick`, not both.',
message:
'<EuiLink> supplied with both `href` and `onClick`; is this intentional? (Valid use cases include programmatic navigation via `onClick` while preserving "Open in new tab" style functionality via `href`.)',
},
],
},
Expand Down
65 changes: 65 additions & 0 deletions packages/eslint-plugin/rules/no_restricted_eui_imports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const DEFAULT_RESTRICTED_IMPORT_PATTERNS = [
{
pattern: '@elastic/eui/dist/eui_theme_*.json',
message:
'For client-side, please use `useEuiTheme` instead. Direct JSON token imports will be removed as per the EUI Deprecation schedule: https://github.com/elastic/eui/issues/1469.',
},
];

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'Disallow deprecated EUI imports.',
category: 'Possible Errors',
recommended: false,
},
schema: [
{
type: 'object',
properties: {
patterns: {
type: 'array',
items: {
type: 'object',
properties: {
pattern: { type: 'string' },
message: { type: 'string' },
},
required: ['pattern'],
additionalProperties: false,
},
uniqueItems: true,
},
},
additionalProperties: false,
},
],
},

create(context) {
const options = context.options[0] || {};
const userPatterns = options.patterns || [];

// Combine the default patterns with the user-defined patterns
const allPatterns = [
...DEFAULT_RESTRICTED_IMPORT_PATTERNS,
...userPatterns,
];

return {
ImportDeclaration(node) {
allPatterns.forEach(({ pattern, message }) => {
const regex = new RegExp(pattern.replace('*', '.*'));
if (regex.test(node.source.value)) {
context.report({
node,
message:
message || `Importing "${node.source.value}" is restricted.`,
});
}
});
},
};
},
};
52 changes: 52 additions & 0 deletions packages/eslint-plugin/rules/no_restricted_eui_imports.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
const { RuleTester } = require('eslint');
const rule = require('./no_restricted_eui_imports');

const ruleTester = new RuleTester({
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 2018,
},
});

ruleTester.run('@elastic/eui/no-restricted-eui-imports', rule, {
valid: [
{
code: "import { EuiButton } from '@elastic/eui';",
},
{
code: "import theme from '@kbn/ui-theme';",
weronikaolejniczak marked this conversation as resolved.
Show resolved Hide resolved
},
],

invalid: [
{
code: "import theme from '@elastic/eui/dist/eui_theme_light.json';",
errors: [
{
message:
'For client-side, please use `useEuiTheme` instead. Direct JSON token imports will be removed as per the EUI Deprecation schedule: https://github.com/elastic/eui/issues/1469.',
},
],
},
{
code: "import theme from '@kbn/ui-theme';",
options: [
{
patterns: [
{
pattern: '@kbn/ui-theme',
message:
'For client-side, please use `useEuiTheme` from `@elastic/eui` instead.',
},
],
},
],
errors: [
{
message:
'For client-side, please use `useEuiTheme` from `@elastic/eui` instead.',
},
],
},
],
});
Loading
Loading