Skip to content

Conversation

@DevSallSa
Copy link
Contributor

@DevSallSa DevSallSa commented Jun 19, 2025

TL;DR

  • Update Angular to v17
  • Replace the deprecated packages with maintained alternatives
  • Configure ESLint and prettier
  • Refactor code to comply with ESLint and prettier rules
  • Fix GitHub Actions job order to resolve lint failure Upgrading to Angular 17 brokes lint command #222.

Context

Based on @OlivierAlbertini ’s PR (#221),
I updated the Angular version to v17 and fixed the following issue: #222.

The lint error was caused by several factors :

  • use of deprecated library : villedemontreal/lint-config which is incompatible with current ESLint version
    • use of deprecated/outdated ESLint rules
    • missing ESLint plugin
  • Version mismatch between eslint, typescript-eslint and angular-eslint packages
  • Incorrect execution order of GitHub Actions jobs (see next paragraph)

Explanation

The Lint job failed because it ran before the build job in GitHub Actions.
The Storybook sub-project depends on the built version of angular-ui and without it, TypeScript cannot resolve certain imports.

For example, the following code will fail during linting:

// content of a storyfile
import { BaoBreadcrumbComponent, BaoHeaderInfoComponent, BaoHeaderInfoModule } from 'angular-ui';

with the following error in GitHub Actions :

/home/runner/work/angular-ui/angular-ui/projects/storybook-angular/src/stories/Header/Header-info.stories.ts
Error:   40:3  error    'BaoHeaderInfoComponent' is an 'error' type that acts as 'any' and overrides all other types in this intersection type      @typescript-eslint/no-redundant-type-constituents

As angular-ui is developed inside this project, it doesn't appear in package.json nor in node_modules.
In that case, Typescript will look in the dist/ folder to find the lib. These types are only available after the build step, as they reside in the dist/ folder but currently as the lint command is ran before the build command, the dist/ folder does not exist.

Step to reproduce in local

  1. Install dependencies : npm ci
  2. Make sure the dist/ folder doesn't exist in the root folder. Delete it otherwise
  3. run npm run lint => The error will occurred
  4. run npm run build && npm run lint => the lint will pass without error (only warnings)

Why now ?

Because of eslint, typescript-eslint and angular-eslint versions supported by Angular v17 which may include breaking changes or enforce stricter best practices.

The solution

To resolve this, we can either :

  • Run the lint command after running the build step
# build-node.yml
- run: npm run build
- run: npm run lint

Or

  • Change all imports to point to the source code.
// before
import { BaoBreadcrumbComponent, BaoHeaderInfoComponent, BaoHeaderInfoModule } from 'angular-ui';

// after
import { BaoBreadcrumbComponent, BaoHeaderInfoComponent, BaoHeaderInfoModule } from 'projects/angular-ui/src/public-api';

This PR implements the first solution to minimize code changes.

@DevSallSa DevSallSa force-pushed the feature/angular-17 branch from 881a26a to fd31e38 Compare June 19, 2025 02:14
@DevSallSa
Copy link
Contributor Author

Hi @OlivierAlbertini,

I've done the changing that you asked in PR #248
I closed the previous PR because it's better to make this PR from my feature branch

@OlivierAlbertini OlivierAlbertini self-requested a review June 19, 2025 13:03
Copy link
Member

@OlivierAlbertini OlivierAlbertini left a comment

Choose a reason for hiding this comment

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

Can you run npm run chromatic this will trigger storybook on this build in order to review visually.

@OlivierAlbertini OlivierAlbertini added the enhancement New feature or request label Jun 19, 2025
* feat: update to angular 17
* feat: update eslint version
* fix: linting errors
* feat: remove deprecated lint package
* feat: remove deprecated lint rules
* fix: eslint rules compliance error
* ci: change node image
* refactor: replace deprecated type
* refactor: replace deprecated type from storybook-angular
* ci: update github actions versions
* fix: broken lint command when upgrading to Angular v17 VilledeMontreal#222
* ci: run build job before lint job to fix linting error

---------

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>
* fix: error when rendering file input component story in storybook
* docs: add comment to improve understanding

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>

---------

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>
@DevSallSa DevSallSa force-pushed the feature/angular-17 branch from 6c5413f to 06c5165 Compare June 20, 2025 18:21
Copy link
Member

@OlivierAlbertini OlivierAlbertini left a comment

Choose a reason for hiding this comment

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

GG

@MaudeLaflamme MaudeLaflamme merged commit 1b5b803 into VilledeMontreal:master Jun 25, 2025
4 checks passed
mt-anna pushed a commit to mt-anna/angular-ui that referenced this pull request Jul 28, 2025
* fix: update to Angular v17 broke linter (VilledeMontreal#2)

* feat: update to angular 17
* feat: update eslint version
* fix: linting errors
* feat: remove deprecated lint package
* feat: remove deprecated lint rules
* fix: eslint rules compliance error
* ci: change node image
* refactor: replace deprecated type
* refactor: replace deprecated type from storybook-angular
* ci: update github actions versions
* fix: broken lint command when upgrading to Angular v17 VilledeMontreal#222
* ci: run build job before lint job to fix linting error

---------

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>

* fix: error when rendering file input component story in storybook  (VilledeMontreal#3)

* fix: error when rendering file input component story in storybook
* docs: add comment to improve understanding

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>

---------

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>

---------

Signed-off-by: DevSallSa <mrsallsa@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants