inveniosoftware / inveniosoftware/react-invenio-app-ils

Technical upgrades

Open
#647 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
6
Forks
24
Avg merge
2d 46m
Merged PRs (30d)
3

Description

The app should go through various technical upgrades.

1. we need to upgrade `react-searchkit` to the latest, and therefore, `axios`. This is not easy, because the NodeJS version might have to be upgraded, leading to a certain number of tests failures. For now, [we had to pin](https://github.com/inveniosoftware/react-invenio-app-ils/commit/761ff7a4f9ea21119634f41acd3185acc9fc404d#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R69) `cheerio` to a specific version, and this should be removed at some point.
2. we should upgrade `NodeJS`. However, this mean upgrading `npm`, and the biggest change is that, by default, newer versions of `npm` than the current v6 install `peerDeps` too. This cause issues. A workaround is to add the param `--legacy-peer-deps` to all npm command to simulate the previous behaviour.
3. `enzyme` is completely abandoned. Anyway, in my opinion, snapshots testing did not turn out to be particularly useful. I propose to remove it entirely and tests too. For new test, we could use [React Testing Library](https://testing-library.com/docs/react-testing-library/intro/).
If it is to be kept, or you the error that `TextEncoder` is not found, [this commit](https://github.com/inveniosoftware/react-searchkit/commit/16dae0123da4d23d3e656b335413d0eec63ca3bd) should be added here too.
4. we could explore removing `craco`, and come back to the default `create react app`. It was added because `create react app` does not support LESS by default, and it is suggested by the [React SemanticUI](https://react.semantic-ui.com/theming#install-required-dependencies) doc. Today, there could be better ways. If this is done, then we need to remove the [aliases](https://github.com/inveniosoftware/react-invenio-app-ils/blob/master/craco.config.js#L13) defined for webpack.
5. we should probably use the [invenio linter rules](https://github.com/CERNDocumentServer/cds-ils/blob/master/ui/.eslintrc.yml).
6. what about upgrading React?
7. partial upgrades done in https://github.com/inveniosoftware/react-invenio-app-ils/pull/629

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reviewing the dependency and npm configuration, the existing Enzyme and snapshot tests, craco.config.js, and .eslintrc.yml. Separate the proposed upgrades and removals into concrete work items before changing anything. Done would require agreed upgrade targets, passing tests, and a decided approach for craco, Enzyme, lint rules, and React.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js, react
Domain
build-system, frontend, testing, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.