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

Jump to bottom

feat: add ESLint, improve types, improve a11y #647

Merged
merged 3 commits into from Jan 18, 2022
Merged

feat: add ESLint, improve types, improve a11y #647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fb3b5c
feat: add ESLint, improve types, improve a11y
CreativeTechGuy Jan 16, 2022
837dac1
restore fsevents to package-lock.json
CreativeTechGuy Jan 16, 2022
0330498
eslint: enable one-liner functions, dangling promises
CreativeTechGuy Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions .eslintignore
View file
Open in desktop
@@ -0,0 +1,9 @@
.cache
docs/dist
docs/search.json
docs/**/*.min.js
dist
examples
node_modules
src/react
scripts
263 changes: 263 additions & 0 deletions .eslintrc.cjs
View file
Open in desktop
@@ -0,0 +1,263 @@
/* eslint-env node */

module.exports = {
plugins: ['@typescript-eslint', 'wc', 'lit', 'lit-a11y', 'chai-expect', 'chai-friendly', 'import'],
extends: [
'eslint:recommended',
'plugin:wc/recommended',
'plugin:wc/best-practice',
'plugin:lit/recommended',
'plugin:lit-a11y/recommended'
],
env: {
es2021: true,
browser: true
},
reportUnusedDisableDirectives: true,
parserOptions: {
sourceType: 'module'
},
overrides: [
{
extends: [
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
'plugin:@typescript-eslint/recommended-requiring-type-checking'
],
parser: '@typescript-eslint/parser',
parserOptions: {
sourceType: 'module',
project: './tsconfig.json',
tsconfigRootDir: __dirname
},
files: ['*.ts'],
rules: {
'default-param-last': 'off',
'@typescript-eslint/default-param-last': 'error',
'no-empty-function': 'off',
'@typescript-eslint/no-empty-function': 'warn',
'no-implied-eval': 'off',
'@typescript-eslint/no-implied-eval': 'error',
'no-invalid-this': 'off',
'@typescript-eslint/no-invalid-this': 'error',
'no-shadow': 'off',
'@typescript-eslint/no-shadow': 'error',
'no-throw-literal': 'off',
'@typescript-eslint/no-throw-literal': 'error',
'no-unused-expressions': 'off',
'@typescript-eslint/prefer-regexp-exec': 'off',
'@typescript-eslint/no-unused-expressions': 'error',
'@typescript-eslint/unbound-method': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-floating-promises': 'off',
'@typescript-eslint/no-misused-promises': [
'error',
{
checksVoidReturn: false
}
],
'@typescript-eslint/consistent-type-assertions': [
'warn',
{
assertionStyle: 'as',
objectLiteralTypeAssertions: 'never'
}
],
'@typescript-eslint/consistent-type-imports': 'warn',
// These are commented out for now as we may want to add them to improve function boundary safety
// "@typescript-eslint/explicit-function-return-type": [
// "error",
// {
// allowTypedFunctionExpressions: true,
// },
// ],
// "@typescript-eslint/explicit-member-accessibility": "warn",
// "@typescript-eslint/explicit-module-boundary-types": "error",
'@typescript-eslint/no-base-to-string': 'error',
'@typescript-eslint/no-confusing-non-null-assertion': 'error',
'@typescript-eslint/no-invalid-void-type': 'error',
'@typescript-eslint/no-require-imports': 'error',
'@typescript-eslint/no-unnecessary-boolean-literal-compare': 'warn',
'@typescript-eslint/no-unnecessary-condition': 'warn',
'@typescript-eslint/no-unnecessary-qualifier': 'warn',
'@typescript-eslint/non-nullable-type-assertion-style': 'warn',
'@typescript-eslint/prefer-for-of': 'warn',
'@typescript-eslint/prefer-optional-chain': 'warn',
'@typescript-eslint/prefer-readonly': 'warn',
'@typescript-eslint/prefer-ts-expect-error': 'warn',
'@typescript-eslint/prefer-return-this-type': 'error',
'@typescript-eslint/prefer-string-starts-ends-with': 'warn',
'@typescript-eslint/require-array-sort-compare': 'error',
'@typescript-eslint/unified-signatures': 'warn',
'@typescript-eslint/array-type': 'warn',
'@typescript-eslint/consistent-type-definitions': ['warn', 'interface'],
'@typescript-eslint/member-delimiter-style': 'warn',
'@typescript-eslint/method-signature-style': 'warn',
'@typescript-eslint/naming-convention': [
'warn',
{
selector: 'default',
format: ['camelCase']
},
{
selector: ['function', 'enumMember', 'property'],
format: ['camelCase', 'PascalCase']
},
{
selector: 'variable',
modifiers: ['const'],
format: ['camelCase', 'PascalCase', 'UPPER_CASE']
},
{
selector: 'typeLike',
format: ['PascalCase']
},
{
selector: 'typeProperty',
format: ['camelCase', 'PascalCase', 'UPPER_CASE']
},
{
selector: 'objectLiteralProperty',
format: null
}
],
'@typescript-eslint/no-extraneous-class': 'error',
'@typescript-eslint/no-parameter-properties': 'error',
'@typescript-eslint/strict-boolean-expressions': [
'error',
{
allowString: false,
allowNumber: false,
allowNullableObject: false
}
]
}
},
{
extends: ['plugin:chai-expect/recommended', 'plugin:chai-friendly/recommended'],
files: ['*.test.ts'],
rules: {
'@typescript-eslint/no-unused-expressions': 'off'
}
}
],
rules: {
'no-template-curly-in-string': 'error',
'array-callback-return': 'error',
'consistent-return': 'error',
curly: 'warn',
'default-param-last': 'error',
eqeqeq: 'error',
'no-constructor-return': 'error',
'no-empty-function': 'warn',
'no-eval': 'error',
'no-extend-native': 'error',
'no-extra-bind': 'error',
'no-floating-decimal': 'error',
'no-implicit-coercion': 'error',
'no-implicit-globals': 'error',
'no-implied-eval': 'error',
'no-invalid-this': 'error',
'no-labels': 'error',
'no-lone-blocks': 'error',
'no-new': 'error',
'no-new-func': 'error',
'no-new-wrappers': 'error',
'no-octal-escape': 'error',
'no-proto': 'error',
'no-return-assign': 'warn',
'no-script-url': 'error',
'no-self-compare': 'warn',
'no-sequences': 'warn',
'no-throw-literal': 'error',
'no-unmodified-loop-condition': 'error',
'no-unused-expressions': 'warn',
'no-useless-call': 'error',
'no-useless-concat': 'error',
'no-useless-return': 'warn',
'prefer-promise-reject-errors': 'error',
radix: 'error',
'require-await': 'error',
'wrap-iife': ['warn', 'inside'],
'no-shadow': 'error',
'no-array-constructor': 'error',
'no-bitwise': 'error',
'no-multi-assign': 'warn',
'no-new-object': 'error',
'no-useless-computed-key': 'warn',
'no-useless-rename': 'warn',
'no-var': 'error',
'prefer-const': 'warn',
'prefer-numeric-literals': 'warn',
'prefer-object-spread': 'warn',
'prefer-rest-params': 'warn',
'prefer-spread': 'warn',
'prefer-template': 'warn',
'no-else-return': 'warn',
'func-names': ['warn', 'never'],
'func-style': ['warn', 'declaration'],
'one-var': ['warn', 'never'],
'operator-assignment': 'warn',
'prefer-arrow-callback': 'warn',
'no-restricted-syntax': [
'warn',
{
selector: "CallExpression[callee.name='String']",
message: "Don't use the String function. Use .toString() instead."
},
{
selector: "CallExpression[callee.name='Number']",
message: "Don't use the Number function. Use parseInt or parseFloat instead."
},
{
selector: "CallExpression[callee.name='Boolean']",
message: "Don't use the Boolean function. Use a strict comparison instead."
}
],
'no-restricted-imports': [
'warn',
{
patterns: [
{
group: ['../*'],
message: 'Usage of relative parent imports is not allowed.'
}
],
paths: [
{
name: '.',
message: 'Usage of local index imports is not allowed.'
},
{
name: './index',
message: 'Import from the source file instead.'
}
]
}
],
'import/no-duplicates': 'warn',
'import/order': [
'warn',
{
groups: ['builtin', 'external', ['parent', 'sibling', 'internal', 'index']],
pathGroups: [
{
pattern: '~/**',
group: 'internal'
},
{
pattern: 'dist/**',
group: 'external'
}
],
alphabetize: {
order: 'asc',
caseInsensitive: true
},
'newlines-between': 'never',
warnOnUnassignedImports: true
}
],
'wc/guard-super-call': 'off'
}
};
2 changes: 1 addition & 1 deletion .vscode/extensions.json
View file
Open in desktop
@@ -1,6 +1,6 @@
{
"recommendations": [
"ms-vscode.vscode-typescript-tslint-plugin",
"dbaeumer.vscode-eslint",
"esbenp.prettier-vscode",
"bierner.lit-html",
"bashmish.es6-string-css",
Expand Down
2 changes: 2 additions & 0 deletions cspell.json
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@
"csspart",
"cssproperty",
"datetime",
"describedby",
"Docsify",
"dropdowns",
"easings",
Expand Down Expand Up @@ -79,6 +80,7 @@
"rgba",
"roadmap",
"Roboto",
"saturationl",
"Schilp",
"Segoe",
"semibold",
Expand Down
19 changes: 11 additions & 8 deletions custom-elements-manifest.config.js
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ import { pascalCase } from 'pascal-case';

const packageData = JSON.parse(fs.readFileSync('./package.json', 'utf8'));
const { name, description, version, author, homepage, license } = packageData;
// eslint-disable-next-line func-style
const noDash = string => string.replace(/^\s?-/, '').trim();
Copy link
Member

Choose a reason for hiding this comment

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

Can we allow this? For simple functions, the overall code is more readable without splitting it out into multiple lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can always add an eslint-disable-next-line comment in some cases. Should I go through and revert any one-liner functions back to arrow functions with disable comments?

Copy link
Member

Choose a reason for hiding this comment

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

I apologize because this is largely coding style and opinion, but making this allowed — at least for short definitions like this one — will really help me out.


export default {
Expand All @@ -13,17 +14,17 @@ export default {
// Append package data
{
name: 'shoelace-package-data',
packageLinkPhase({ customElementsManifest, context }) {
packageLinkPhase({ customElementsManifest }) {
customElementsManifest.package = { name, description, version, author, homepage, license };
}
},

// Parse custom jsDoc tags
{
name: 'shoelace-custom-tags',
analyzePhase({ ts, node, moduleDoc, context }) {
analyzePhase({ ts, node, moduleDoc }) {
switch (node.kind) {
case ts.SyntaxKind.ClassDeclaration:
case ts.SyntaxKind.ClassDeclaration: {
const className = node.name.getText();
const classDoc = moduleDoc?.declarations?.find(declaration => declaration.name === className);
const customTags = ['animation', 'dependency', 'since', 'status'];
Expand All @@ -39,8 +40,8 @@ export default {
});
});

const parsed = parse(customComments + '\n */');
parsed[0].tags?.map(t => {
claviska marked this conversation as resolved.
Show resolved Hide resolved
const parsed = parse(`${customComments}\n */`);
parsed[0].tags?.forEach(t => {
switch (t.tag) {
// Animations
case 'animation':
Expand Down Expand Up @@ -80,23 +81,25 @@ export default {
});
}
});
}
}
}
},

{
name: 'shoelace-react-event-names',
analyzePhase({ ts, node, moduleDoc, context }) {
analyzePhase({ ts, node, moduleDoc }) {
switch (node.kind) {
case ts.SyntaxKind.ClassDeclaration:
case ts.SyntaxKind.ClassDeclaration: {
const className = node.name.getText();
const classDoc = moduleDoc?.declarations?.find(declaration => declaration.name === className);

if (classDoc?.events) {
classDoc.events.map(event => {
classDoc.events.forEach(event => {
event.reactName = `on${pascalCase(event.name)}`;
});
}
}
}
}
}
Expand Down