inveniosoftware / inveniosoftware/react-invenio-app-ils
Technical upgrades
- 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