Skip to content

upgrade helix-front to Angular 13#2054

Closed
micahstubbs wants to merge 42 commits into
apache:masterfrom
micahstubbs:micahstubbs/2053/helix-front-angular-13
Closed

upgrade helix-front to Angular 13#2054
micahstubbs wants to merge 42 commits into
apache:masterfrom
micahstubbs:micahstubbs/2053/helix-front-angular-13

Conversation

@micahstubbs

@micahstubbs micahstubbs commented Apr 19, 2022

Copy link
Copy Markdown
Contributor

Issues

  • My PR addresses the following Helix issues and references them in the PR description:

Fix #2053 #1573 #1574

Description

  • Here are some details about my PR, including screenshots of any UI changes:

This PR upgrades helix-front to Angular 13. In order to do this, it upgrades many other dependencies, and makes necessary code changes in response to breaking changes from dependencies. This PR also fixes or ignores TypeScript and Linting errors that arise when upgrading type-checking and linting tools.

Tests

  • yarn build runs successfully
  • yarn test runs and passes
  • yarn e2e runs and passes

Currently work in progress. There is a known build failure in the yarn build script, which @micahstubbs is currently debugging.

current blocking issue: #2054 (comment)

Changes that Break Backward Compatibility (Optional)

N/A

Documentation (Optional)

No new functionality, only dependency upgrades to unblock development for future new functionality.

@micahstubbs micahstubbs marked this pull request as draft April 19, 2022 19:50
@micahstubbs

micahstubbs commented Apr 19, 2022

Copy link
Copy Markdown
Contributor Author

This PR is currently work in progress. There is a known build failure in the yarn build script.

EDIT: now fixed by temporarily disabling css minification in prod build.

current blocking issue: #2054 (comment)

documented by two existing Angular community StackOverflow questions:

I think this is an issue with the way that the scss styles files are being bundled and converted to css. My guess is that something with sass changed across the version updates that requires a config or code change that this PR doesn't have yet. Any advice on this problem will be most welcome. I am actively researching this build issue.

@micahstubbs

micahstubbs commented Apr 19, 2022

Copy link
Copy Markdown
Contributor Author

Here is the commit log from my original branch. This is helpful if you want to see the path I took to upgrade the project. At a high level, I upgraded Angular one major version at a time, from Angular 5 to Angular 13. Along the way, I upgraded other dependencies as needed.

commit log up until PR creation
prefer yarn
HttpModule --> HttpClientModule
preserveWhitespaces: off
use two dashes for args to ng commands in package.json
@angular/compiler: "6.1.10",
install dart sass, set node@v6
update to angular@6
npm install @angular/cli@7 @angular/core@7
npm install @angular/material@7
npm install @angular/animations@7
npm install @angular/cdk@7 @angular/common@7 @angular/compiler@7 @angular/forms@7 @angular/http@7 @angular/platform-browser@7 @angular/platform-browser-dynamic@7
npm install rxjs@6
update to angular@8
ng update @angular/cdk@9 --force
ng update @angular/cdk@9 --force
remove ~ from angular/material imports in scss files
ng update @angular/material@9
remove entryComponents
npx @angular/cli@10 update @angular/core@10 @angular/cli@10
npx @angular/cli@10 update @angular/material@10
npm install --save-dev typescript@~3.9
npm install tslint@6 tslib@2
stop using styleExt
npx @angular/cli@11 update @angular/core@11 @angular/cli@11 --force
npx @angular/cli@11 update @angular/material@11
npm install --save-dev typescript@4
migrate to angular.json.
npx @angular/cli@12 update @angular/material@12
npx @angular/cli@13 update @angular/core@13 @angular/cli@13 --force
npx @angular/cli@13 update @angular/material@13 --force
npm install rxjs@^7.4.0
prefer yarn, add yarn.lock
migrate from @angular/http to @angular/common/http
use latest version of core-js
remove unknown compiler option preserveWhiteSpacesg
ng add @angular-eslint/schematics
yarn add @typescript-eslint/eslint-plugin-tslint eslint-config-prettier --dev
commit .eslintrc.js eslint file generate from tsli nt along with log file of rules that could not be converted
rm tslint.json
Revert "rm tslint.json"
yarn lint
remove root: true from .eslintrc.js
add yarn lint:errors:only script
convert all failing linting rules to warn, fix later in another PR for reviewer ease
upgrade to @angular/flex-layout@13.0.0-beta.38
upgrade to @swimlane/ngx-charts@20.1.0
upgrade @types/node@17.0.23
remove unused dependency codelyzer
upgrade @swimlane/ngx-datatable@20.0.0
upgrade to ngx-clipboard@15.1.0
upgrade to ngx-vis@3.1.0
add peer deps for ngx-vis
add peer deps for ngx-vis dep vis-network
add peer deps for ngx-vis dep vis-timeline
update styles to angular13 syntax
ng update @angular-eslint/schematics
yarn add --dev @angular-devkit/build-angular
yarn add --dev @angular/compiler-cli
omit --prod from build command since it is the default
update reference from .angular-cli.json to angular.json
move --aot to production build options
remove outDir
add lint config to angular.json based on SAP spartacus angular.json
ng update @angular-eslint/schematics
regenerate yarn.lock
use ngx-json-viewer version that is upgraded to angular13
pin latest commit of ngx-datatable that supports angular 13
upgrade ngx-vis dependency
upgrade to angulartics2@12.0.0
migrate from ngx-dag to @swimlane/ngx-graph
fix css property border-radius typo
fix sass undefined variable error
try a different way to specify latest commit for @swimlane/ngx-datatable
set swimlane/ngx-datable#master
add types yarn scripts
add missing space in scss import
prettier ignore scss files
add type declaration for third party library @swimlane/ngx-datatable
specify a type for ModuleWithProviders
AppRoutingModule --> appRoutingModule
Angulartics2Piwik --> Angulartics2Matomo
add comment about Piwik --> Matomo rename in angulartics2
fix angular material imports
fix rxjs imports and tune linting rules
turn off eslint quotes rule and add for now noop prettier config to document project preference for single quoted strings
lint and fix type errors in user controller
fix linting and typescript errors in helix controller
skipLibCheck: true do not type check node_modules
fix all remaining typescript errors
yarn lint:fix
update material theme defs
fix mat color palette references
update mat.get-color-from-palette and mat.define-palette usage
update path to ngx-datatable styles
fix paths for ngx-datatable styles
import scss file material theme for ngx-datatable
upgrade to sass@1.50.0
import style from source for ngx-datatable
move library type declarations in app root client dir
rm top level types dir
replace material ease-out transition with css equivalent
add helpful comment for transition
migrate styles to use new mat.-palette for getting colors
get hi-warn color directly from theme
get more colors directly from theme
one more place to get color directly from theme
use relative path to theme
convert remaining theme import to @use from relative path

Comment thread helix-front/angular.json
"includePaths": [
"client/"
]
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is recommended by this StackOverflow Answer from earlier today.

However, I still see this error:

./client/main.ts - Error: Module build failed (from ./node_modules/@ngtools/webpack/src/ivy/index.js):
Error: Transform failed with 1 error:
helix/helix-front/client/app/cluster/cluster-list/cluster-list.component.scss:17:100: ERROR: Unterminated string token
    at helix/helix-front/node_modules/@ngtools/webpack/src/ivy/loader.js:81:18
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@micahstubbs

Copy link
Copy Markdown
Contributor Author

@micahstubbs

Copy link
Copy Markdown
Contributor Author

Comment thread helix-front/tslint.json
"no-unused-expression": true,
"no-use-before-declare": true,
"no-var-keyword": true,
"object-literal-sort-keys": false,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

probably should remove since we have tslint-to-eslint-config.log

Comment thread helix-front/client/app/cluster/cluster-list/cluster-list.component.scss Outdated
@junkaixue

Copy link
Copy Markdown
Contributor

Why we need to do this upgrade?

@micahstubbs

micahstubbs commented Apr 26, 2022

Copy link
Copy Markdown
Contributor Author

hi @junkaixue, thanks for the question.

Two acute problems motivate upgrading the web app helix-front to the latest version of the Angular UI framework (Angular 13 at this writing).

  1. node-sass dependency doesn't support M1 MacBooks.

  2. Several other dependencies have security vulnerabilities that we are unable to fix without a holistic upgrade, since often the new major version of the insecure dependency that fixes the security vulnerability requires us to upgrade to a corresponding new major version of another old-but-apparently-secure dependency.

for example:

> Notes: Vulnerability found: ansi-regex is vulnerable to Inefficient Regular Expression Complexity remediation: Upgrade ansi-regex from 4.1.1 to 5.0.1 to fix the vulnerability.

Other developer ergonomics issues also motivate the upgrade:

  • helix-front will not build with current nodejs or npm.
  • helix-front may not build with the yarn package manager.
  • helix-front builds very slowly since, it requires the developer to compile the old node-sass. This compilation often fails, for difficult to debug reasons.
  • helix-front --> node-sass --> node-gyp as-is requires Python 2, which is unsupported in many places. It doesn't make sense for a JavaScript project like this to have a direct dependency on an old major version of Python.
  • it's difficult to research problems or get community support for Angular 5 and other old dependencies, since most community resources now refer to a newer version of Angular.

You can read more in #2053 😄

@micahstubbs

Copy link
Copy Markdown
Contributor Author

Closed in favor of #2078, for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upgrade helix-front to Angular 6.1

2 participants