Bug #17782
closedMigrate WB2 app from react-scripts-ts to create-react-app
Description
Currently we have lots of old packages that cannot (or shouldn't) be upgraded because they're indirect dependencies of the abandoned react-scripts-ts
package.
For some time now, the create-react-app
package supports Typescripts so it should be able to upgrade the app to it, unlocking further security package upgrades.
Also, we're probably missing on the newer testing/linting features because of this.
Here's one guide on how to do the migration: https://vincenttunru.com/migrate-create-react-app-typescript-to-create-react-app/
Updated by Lucas Di Pentima over 3 years ago
- Target version changed from 2021-06-23 sprint to 2021-07-07 sprint
Updated by Lucas Di Pentima over 3 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima over 3 years ago
Updates at arvados-workbench2|e2914bad - branch 17782-react-scripts-ts-migration
Test run: developer-tests-workbench2: #442
Updates¶
- Migrates from
react-scripts-ts
toreact-scripts
3.4.4 - Fixes some cypress tests.
- Changes
node
requirement to be version 12. (CI pending to be updated to be able to test this on the development pipeline) - Updates forced package upgrades (from
resolutions
key inpackages.json
) to let the canonical deps to correctly be resolved:yarn audit
onmain
branch: Severity: 100 Low | 45 Moderate | 1 High (from 1928 packages)yarn audit
on this branch: Severity: 4 Low | 5 Moderate | 1 High (from 2073 packages)
Notes¶
Lots of changes in linter rule strictness, TS compilation, and other environmental features required major changes in the code base, updating module imports, type-related code changes, and other tasks.
I recommend re-creating the node_modules/
directory from scratch as I have seen issues when doing an upgrade from a previous state.
I tried to go from react-scripts
3.4.4 to 4.0.0 but the number of errors produced talked me out of that, we can go into that adventure in the future.
Updated by Lucas Di Pentima over 3 years ago
- Target version changed from 2021-07-07 sprint to 2021-07-21 sprint
Updated by Lucas Di Pentima over 3 years ago
Updates at commit:9d7aa0bee - branch 17782-nodejs-update
(arvados repo)
- Changes
arvados-server install
'snodejs
requirements to be version 12.
Updates at arvados-workbench2|43fff13 (wb2 branch)
- Adds an
arvados-server install
run as a prerequisite to runyarn install
on the Makefile, so the correctnodejs
version is available.
Now Jenkins is able to run the tests: developer-tests-workbench2: #445
Updated by Lucas Di Pentima over 3 years ago
Running arvados
tests with nodejs v12 from arvados-server install
updates: developer-run-tests: #2574
Successful wb1 integration tests run: developer-run-tests-apps-workbench-integration: #2731
Updated by Nico César over 3 years ago
I think we should merge commit:9d7aa0bee - branch 17782-nodejs-update (arvados repo)
make workbech2-build-image does:
Step 7/7 : RUN git clone https://git.arvados.org/arvados.git && cd arvados && go mod download && go run ./cmd/arvados-server install -type test && cd .. && rm -rf arvados && apt-get clean ---> Running in 6087c482804d Cloning into 'arvados'...
also noticed that you need to add git to the docker/Dockerfile if you change to debian:buster (this is not included)
Updated by Lucas Di Pentima over 3 years ago
Merged 17782-nodejs-update
at commit:b59aca2b57068547827d7b681670fb197ad0e144
Updated by Nico César over 3 years ago
I tried the Docker image and git was missing, once I added it to apt-get there was an issue with yarn. This could be the case that /var/lib/arvados/node-v12.22.2-linux-x64/bin/yarn is not in the PATH, here is my output
docker run -ti -v$PWD:$PWD -p 3000:3000 -w$PWD workbench2-build /bin/bash root@3e74c0c675f6:/home/nico/jobs/curii/arvados-workbench2# npm install --global yarn> yarn@1.22.10 preinstall /var/lib/arvados/node-v12.22.2-linux-x64/lib/node_modules/yarn > :; (node ./preinstall.js > /dev/null 2>&1 || true)/var/lib/arvados/node-v12.22.2-linux-x64/bin/yarn -> /var/lib/arvados/node-v12.22.2-linux-x64/lib/node_modules/yarn/bin/yarn.js /var/lib/arvados/node-v12.22.2-linux-x64/bin/yarnpkg -> /var/lib/arvados/node-v12.22.2-linux-x64/lib/node_modules/yarn/bin/yarn.js + yarn@1.22.10 added 1 package in 1.325s root@3e74c0c675f6:/home/nico/jobs/curii/arvados-workbench2# yarn install bash: yarn: command not found root@3e74c0c675f6:/home/nico/jobs/curii/arvados-workbench2# /var/lib/arvados/node-v12.22.2-linux-x64/bin/yarn yarn install v1.22.10 [1/4] Resolving packages... warning Resolution field "css-what@5.0.1" is incompatible with requested version "css-what@^3.2.1"
after that it compiled with the following errors:
Compiled with warnings. ./src/lib/cwl-svg/graph/edge.ts Line 13:16: 'sourceSide' is assigned a value but never used @typescript-eslint/no-unused-vars Line 14:16: 'destSide' is assigned a value but never used @typescript-eslint/no-unused-vars Line 68:16: 'sourceSide' is assigned a value but never used @typescript-eslint/no-unused-vars Line 69:16: 'destSide' is assigned a value but never used @typescript-eslint/no-unused-vars ./src/lib/cwl-svg/utils/html-utils.ts Line 13:46: Unnecessary escape character: \/ no-useless-escape ./src/lib/cwl-svg/plugins/selection/selection.ts Line 149:13: Expected a conditional expression and instead saw an assignment no-cond-assign Line 154:20: Expected a conditional expression and instead saw an assignment no-cond-assign ./src/lib/cwl-svg/graph/workflow.ts Line 194:17: 'arrangeNecessary' is assigned a value but never used @typescript-eslint/no-unused-vars ./src/lib/cwl-svg/utils/geometry.ts Line 19:36: Use 'as SVGGElement' instead of '<SVGGElement>' @typescript-eslint/consistent-type-assertions ./src/services/collection-service/collection-service-files-response.ts Line 23:41: Unnecessary escape character: \/ no-useless-escape ./src/store/dialog/with-dialog.ts Line 23:9: 'P' is defined but never used @typescript-eslint/no-unused-vars ./src/lib/cwl-svg/plugins/port-drag/port-drag.ts Line 268:29: Use 'as any' instead of '<any>' @typescript-eslint/consistent-type-assertions ./src/views/workbench/workbench-loading-screen.tsx Line 29:9: img elements must have an alt prop, either with meaningful text, or an empty string for decorative images jsx-a11y/alt-text ./src/lib/cwl-svg/plugins/deletion/deletion.ts Line 32:28: Unexpected mix of '&&' and '||' no-mixed-operators Line 32:47: Unexpected mix of '&&' and '||' no-mixed-operators ./src/lib/cwl-svg/utils/dom-events.ts Line 34:39: Function declared in a loop contains unsafe references to variable(s) 'target' no-loop-func ./src/lib/cwl-svg/plugins/edge-hover/edge-hover.ts Line 43:12: The function binding is unnecessary no-extra-bind Line 49:12: The function binding is unnecessary no-extra-bind ./src/views/workbench/fed-login.tsx Line 45:32: <iframe> elements must have a unique title property jsx-a11y/iframe-has-title ./src/common/url.ts Line 6:37: Unnecessary escape character: \[ no-useless-escape ./src/components/tree/virtual-tree.tsx Line 87:25: '_' is defined but never used @typescript-eslint/no-unused-vars Line 177:32: '_' is defined but never used @typescript-eslint/no-unused-vars Search for the keywords to learn more about each warning. To ignore, add // eslint-disable-next-line to the line before.
tested with the following public/config.json
{ "API_HOST": "ce8i5.arvadosapi.com", "FILE_VIEWERS_CONFIG_URL": "file-viewers-example.json" }
and all this seems to be working.
Updated by Lucas Di Pentima over 3 years ago
Updates at arvados-workbench2|c32095f0
Test run: developer-tests-workbench2: #446
- Changes back the docker base image from
debian:buster
tonode:12.22.3-buster
as it also installs other needed packages likeyarn
- Fixes the majority of the linter warnings, there're 2 or 3 cases where I just asked the linter to just shut up because I'm not sure what's the correct solution.
Couldn't test the docker changes yet as I'm having issues with my home connection and it never passes the git clone
stage, will retry tomorrow.
Updated by Lucas Di Pentima over 3 years ago
Merged main
with their corresponding fixes at arvados-workbench2|38d27e9
Test run: developer-tests-workbench2: #447
Updated by Nico César over 3 years ago
tested docker and standalone commit:38d27e9|arvados-workbench2
LGTM! ready to merge
Updated by Lucas Di Pentima over 3 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados:arvados-workbench2|b6ac7fe88d347582d39fffa002e300af222c578f.