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

Introduce custom ESLint rules to apply and enforce new themed component selector convention #2865

Merged
merged 37 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3937be1
Custom ESLint rules to enforce new ThemedComponent selector convention
ybnd Mar 14, 2024
13e9808
Don't enforce ThemedComponent selectors in test HTML
ybnd Mar 14, 2024
9a27db3
Lint e2e tests, enforce selectors
ybnd Mar 14, 2024
ae50780
Enable linting for the lint plugins
ybnd Mar 14, 2024
e83a0cd
Fix lint lint issues
ybnd Mar 14, 2024
b0758c2
Enforce plugin structure and generate documentation
ybnd Mar 14, 2024
6e22b53
Make rules more type-safe
ybnd Mar 15, 2024
5685745
Workaround/document edge case where node can't be found by token
ybnd Mar 21, 2024
e40b6ae
Update plugins to support standalone components
ybnd Mar 21, 2024
6051b82
Automatically migrate to new themeable component convention
ybnd Mar 28, 2024
0b9741d
Manual fix: update i/o for ThemedConfigurationSearchPageComponent
ybnd Mar 28, 2024
762e461
Manual fix: sync removed imports between tests and components
ybnd Mar 28, 2024
a6e0930
Manual fix: use base components in unit test templates
ybnd Mar 28, 2024
2c68589
Move generated documentation to docs/lint/
ybnd Mar 29, 2024
515e5f0
Improve documentation
ybnd Mar 29, 2024
08a5443
Add generated docs to source
ybnd Mar 29, 2024
c4b32fe
Explain themed-component-classes rule
ybnd Mar 29, 2024
b99d903
Fix stray Angular 17 dependency
ybnd Apr 3, 2024
ac48a49
Merge remote-tracking branch 'origin/main' into poc-eslint-plugin-aut…
ybnd Apr 4, 2024
1b11356
Merge remote-tracking branch 'origin/main' into poc-eslint-plugin-aut…
ybnd Apr 4, 2024
2e9acc3
Merge remote-tracking branch 'origin/main' into poc-eslint-plugin-aut…
ybnd Apr 17, 2024
efec114
Auto-migrate to fix new ds-themed-* usages
ybnd Apr 17, 2024
14a19b2
Upgrade TSESLint to support TypeScript 5.3.3
ybnd Apr 18, 2024
dc1053e
Resolve lint issues due to new default rules
ybnd Apr 18, 2024
eec052f
Fix Community stats page selector
ybnd Apr 18, 2024
f1c25ee
Fix overindented decorated properties
ybnd Apr 18, 2024
63090b5
Update README
ybnd Apr 18, 2024
bb10b80
Build ESLint plugins after yarn install
ybnd Apr 18, 2024
858d69c
Merge remote-tracking branch 'origin/main' into poc-eslint-plugin-aut…
ybnd Apr 22, 2024
80be4fc
Skip build:lint if installed without source
ybnd Apr 24, 2024
728b561
Optional postinstall script should be cross-platform
ybnd Apr 24, 2024
671e9f1
Move ESLint plugins to dev dependencies
ybnd Apr 24, 2024
145a0a0
Fix lint plugins & tests on Windows
ybnd Apr 25, 2024
348dcc4
Generated docs should use Unix line endings on Windows
ybnd Apr 25, 2024
3a0c964
Upgrade @angular-eslint & get rid of warning in tests
ybnd Apr 25, 2024
c9d9e12
Merge remote-tracking branch 'origin/main' into poc-eslint-plugin-aut…
ybnd Apr 30, 2024
a48d199
Autofix: remove unneeded base component imports
ybnd Apr 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
43 changes: 37 additions & 6 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@
"eslint-plugin-jsonc",
"eslint-plugin-rxjs",
"eslint-plugin-simple-import-sort",
"eslint-plugin-import-newlines"
"eslint-plugin-import-newlines",
"eslint-plugin-jsonc",
"dspace-angular-ts",
"dspace-angular-html"
],
"ignorePatterns": [
"lint/test/fixture"
],
"overrides": [
{
Expand All @@ -21,7 +27,8 @@
"parserOptions": {
"project": [
"./tsconfig.json",
"./cypress/tsconfig.json"
"./cypress/tsconfig.json",
"./lint/tsconfig.json"
],
"createDefaultProgram": true
},
Expand All @@ -38,7 +45,10 @@
"error",
2,
{
"SwitchCase": 1
"SwitchCase": 1,
"ignoredNodes": [
"ClassBody.body > PropertyDefinition[decorators.length > 0] > .key"
]
}
],
"max-classes-per-file": [
Expand Down Expand Up @@ -212,6 +222,15 @@
"@typescript-eslint/no-unsafe-return": "off",
"@typescript-eslint/restrict-template-expressions": "off",
"@typescript-eslint/require-await": "off",
"@typescript-eslint/no-base-to-string": [
"error",
{
"ignoredTypeNames": [
"ResourceType",
"Error"
]
}
],

"deprecation/deprecation": "warn",

Expand All @@ -238,7 +257,12 @@
"method"
],

"rxjs/no-nested-subscribe": "off" // todo: go over _all_ cases
"rxjs/no-nested-subscribe": "off", // todo: go over _all_ cases

// Custom DSpace Angular rules
"dspace-angular-ts/themed-component-classes": "error",
"dspace-angular-ts/themed-component-selectors": "error",
"dspace-angular-ts/themed-component-usages": "error"
}
},
{
Expand All @@ -253,7 +277,10 @@
"createDefaultProgram": true
},
"rules": {
"prefer-const": "off"
"prefer-const": "off",

// Custom DSpace Angular rules
"dspace-angular-ts/themed-component-usages": "error"
}
},
{
Expand All @@ -262,7 +289,11 @@
],
"extends": [
"plugin:@angular-eslint/template/recommended"
]
],
"rules": {
// Custom DSpace Angular rules
"dspace-angular-html/themed-component-usages": "error"
}
},
{
"files": [
Expand Down
5 changes: 4 additions & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@
*.css eol=lf
*.scss eol=lf
*.html eol=lf
*.svg eol=lf
*.svg eol=lf

# Generated documentation should have LF line endings to reduce git noise
docs/lint/**/*.md eol=lf
8 changes: 7 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,14 @@ jobs:
- name: Install Yarn dependencies
run: yarn install --frozen-lockfile

- name: Build lint plugins
run: yarn run build:lint

- name: Run lint plugin tests
run: yarn run test:lint:nobuild

- name: Run lint
run: yarn run lint --quiet
run: yarn run lint:nobuild --quiet

- name: Check for circular dependencies
run: yarn run check-circ-deps
Expand Down
2 changes: 2 additions & 0 deletions angular.json
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@
"options": {
"lintFilePatterns": [
"src/**/*.ts",
"cypress/**/*.ts",
"lint/**/*.ts",
"src/**/*.html",
"src/**/*.json5"
]
Expand Down
26 changes: 13 additions & 13 deletions cypress/e2e/login-modal.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,31 @@ import { testA11y } from 'cypress/support/utils';
const page = {
openLoginMenu() {
// Click the "Log In" dropdown menu in header
cy.get('ds-themed-header [data-test="login-menu"]').click();
cy.get('ds-header [data-test="login-menu"]').click();
},
openUserMenu() {
// Once logged in, click the User menu in header
cy.get('ds-themed-header [data-test="user-menu"]').click();
cy.get('ds-header [data-test="user-menu"]').click();
},
submitLoginAndPasswordByPressingButton(email, password) {
// Enter email
cy.get('ds-themed-header [data-test="email"]').type(email);
cy.get('ds-header [data-test="email"]').type(email);
// Enter password
cy.get('ds-themed-header [data-test="password"]').type(password);
cy.get('ds-header [data-test="password"]').type(password);
// Click login button
cy.get('ds-themed-header [data-test="login-button"]').click();
cy.get('ds-header [data-test="login-button"]').click();
},
submitLoginAndPasswordByPressingEnter(email, password) {
// In opened Login modal, fill out email & password, then click Enter
cy.get('ds-themed-header [data-test="email"]').type(email);
cy.get('ds-themed-header [data-test="password"]').type(password);
cy.get('ds-themed-header [data-test="password"]').type('{enter}');
cy.get('ds-header [data-test="email"]').type(email);
cy.get('ds-header [data-test="password"]').type(password);
cy.get('ds-header [data-test="password"]').type('{enter}');
},
submitLogoutByPressingButton() {
// This is the POST command that will actually log us out
cy.intercept('POST', '/server/api/authn/logout').as('logout');
// Click logout button
cy.get('ds-themed-header [data-test="logout-button"]').click();
cy.get('ds-header [data-test="logout-button"]').click();
// Wait until above POST command responds before continuing
// (This ensures next action waits until logout completes)
cy.wait('@logout');
Expand Down Expand Up @@ -102,10 +102,10 @@ describe('Login Modal', () => {
page.openLoginMenu();

// Registration link should be visible
cy.get('ds-themed-header [data-test="register"]').should('be.visible');
cy.get('ds-header [data-test="register"]').should('be.visible');

// Click registration link & you should go to registration page
cy.get('ds-themed-header [data-test="register"]').click();
cy.get('ds-header [data-test="register"]').click();
cy.location('pathname').should('eq', '/register');
cy.get('ds-register-email').should('exist');

Expand All @@ -119,10 +119,10 @@ describe('Login Modal', () => {
page.openLoginMenu();

// Forgot password link should be visible
cy.get('ds-themed-header [data-test="forgot"]').should('be.visible');
cy.get('ds-header [data-test="forgot"]').should('be.visible');

// Click link & you should go to Forgot Password page
cy.get('ds-themed-header [data-test="forgot"]').click();
cy.get('ds-header [data-test="forgot"]').click();
cy.location('pathname').should('eq', '/forgot');
cy.get('ds-forgot-email').should('exist');

Expand Down
8 changes: 4 additions & 4 deletions cypress/e2e/search-navbar.cy.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
const page = {
fillOutQueryInNavBar(query) {
// Click the magnifying glass
cy.get('ds-themed-header [data-test="header-search-icon"]').click();
cy.get('ds-header [data-test="header-search-icon"]').click();
// Fill out a query in input that appears
cy.get('ds-themed-header [data-test="header-search-box"]').type(query);
cy.get('ds-header [data-test="header-search-box"]').type(query);
},
submitQueryByPressingEnter() {
cy.get('ds-themed-header [data-test="header-search-box"]').type('{enter}');
cy.get('ds-header [data-test="header-search-box"]').type('{enter}');
},
submitQueryByPressingIcon() {
cy.get('ds-themed-header [data-test="header-search-icon"]').click();
cy.get('ds-header [data-test="header-search-icon"]').click();
},
};

Expand Down
4 changes: 4 additions & 0 deletions docs/lint/html/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[DSpace ESLint plugins](../../../lint/README.md) > HTML rules
_______

- [`dspace-angular-html/themed-component-usages`](./rules/themed-component-usages.md): Themeable components should be used via the selector of their `ThemedComponent` wrapper class
110 changes: 110 additions & 0 deletions docs/lint/html/rules/themed-component-usages.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
[DSpace ESLint plugins](../../../../lint/README.md) > [HTML rules](../index.md) > `dspace-angular-html/themed-component-usages`
_______

Themeable components should be used via the selector of their `ThemedComponent` wrapper class

This ensures that custom themes can correctly override _all_ instances of this component.
The only exception to this rule are unit tests, where we may want to use the base component in order to keep the test setup simple.


_______

[Source code](../../../../lint/src/rules/html/themed-component-usages.ts)

### Examples


#### Valid code

##### use no-prefix selectors in HTML templates

```html
<ds-test-themeable/>
<ds-test-themeable></ds-test-themeable>
<ds-test-themeable [test]="something"></ds-test-themeable>
```

##### use no-prefix selectors in TypeScript templates

```html
@Component({
template: '<ds-test-themeable></ds-test-themeable>'
})
class Test {
}
```

##### use no-prefix selectors in TypeScript test templates

Filename: `lint/test/fixture/src/test.spec.ts`

```html
@Component({
template: '<ds-test-themeable></ds-test-themeable>'
})
class Test {
}
```

##### base selectors are also allowed in TypeScript test templates

Filename: `lint/test/fixture/src/test.spec.ts`

```html
@Component({
template: '<ds-base-test-themeable></ds-base-test-themeable>'
})
class Test {
}
```




#### Invalid code &amp; automatic fixes

##### themed override selectors are not allowed in HTML templates

```html
<ds-themed-test-themeable/>
<ds-themed-test-themeable></ds-themed-test-themeable>
<ds-themed-test-themeable [test]="something"></ds-themed-test-themeable>
```
Will produce the following error(s):
```
Themeable components should be used via their ThemedComponent wrapper's selector
Themeable components should be used via their ThemedComponent wrapper's selector
Themeable components should be used via their ThemedComponent wrapper's selector
```

Result of `yarn lint --fix`:
```html
<ds-test-themeable/>
<ds-test-themeable></ds-test-themeable>
<ds-test-themeable [test]="something"></ds-test-themeable>
```


##### base selectors are not allowed in HTML templates

```html
<ds-base-test-themeable/>
<ds-base-test-themeable></ds-base-test-themeable>
<ds-base-test-themeable [test]="something"></ds-base-test-themeable>
```
Will produce the following error(s):
```
Themeable components should be used via their ThemedComponent wrapper's selector
Themeable components should be used via their ThemedComponent wrapper's selector
Themeable components should be used via their ThemedComponent wrapper's selector
```

Result of `yarn lint --fix`:
```html
<ds-test-themeable/>
<ds-test-themeable></ds-test-themeable>
<ds-test-themeable [test]="something"></ds-test-themeable>
```



6 changes: 6 additions & 0 deletions docs/lint/ts/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[DSpace ESLint plugins](../../../lint/README.md) > TypeScript rules
_______

- [`dspace-angular-ts/themed-component-classes`](./rules/themed-component-classes.md): Formatting rules for themeable component classes
- [`dspace-angular-ts/themed-component-selectors`](./rules/themed-component-selectors.md): Themeable component selectors should follow the DSpace convention
- [`dspace-angular-ts/themed-component-usages`](./rules/themed-component-usages.md): Themeable components should be used via their `ThemedComponent` wrapper class
Loading
Loading