[FE-218] Update frontend deps #18524
Conversation
ghost
added
1 Bug
3 UI
ghost
requested review from
ghost
self-assigned this
|
Jenkins: https://jenkins01.arangodb.biz/view/PR/job/arangodb-matrix-pr/28706/ EDIT: This Jenkins run seems to have killed itself during jslint. |
...aardvark/APP/react/src/views/collections/indices/addIndex/invertedIndex/FieldBreadcrumbs.tsx
Show resolved
Hide resolved
js/apps/system/_admin/aardvark/APP/frontend/scss/font-awesome/_variables.scss
Show resolved
Hide resolved
The extra aliases and css loader seem to have no effect, so might as well remove them.
| var url = 'cluster/amICoordinator'; | ||
| if (frontendConfig.react) { | ||
| url = arangoHelper.databaseUrl('/_admin/aardvark/cluster/amICoordinator'); | ||
| } |
There was a problem hiding this comment.
These are no longer necessary because they were workarounds for the buggy dev proxy. I've fixed the proxy, see below.
| // react unmounting | ||
| ReactDOM.unmountComponentAtNode(document.getElementById('content-react')); | ||
| const reactRoot = document.getElementById('content-react'); | ||
| if (reactRoot) ReactDOM.unmountComponentAtNode(reactRoot); |
There was a problem hiding this comment.
The DOM node isn't initially loaded in dev mode and unmountComponentAtNode no longer likes that. This is probably redundant in prod, but here we are.
There was a problem hiding this comment.
@pluma4345 there's another ReactDOM.unmount below this, maybe that needs the same check?
There was a problem hiding this comment.
No, that one is fine, apparently. I don't know what exactly causes the first one to sometimes error out but it's only the first one. I'll leave it as is for now.
| @@ -0,0 +1 @@ | |||
| REACT_APP_ARANGODB_CONFIG_PATH="config.js" No newline at end of file | |||
There was a problem hiding this comment.
This is still needed although this value is the same in devel and prod now because CRA otherwise tries to resolve the path if we use it in index.html verbatim. If someone has a good workaround to avoid this, I'm all ears.
| app.use( | ||
| routes, | ||
| proxy({ | ||
| createProxyMiddleware(["/_db/**", "!/_db/*/_admin/aardvark/index.html"], { |
There was a problem hiding this comment.
This now allows us to proxy all requests on port 3000 instead of having to deal with cross-origin request. All ArangoDB routes start with /_db but we need a special case to make sure the index.html is served by CRA. This also means a lot of weird bugs in the devel version of the frontend are now fixed because we no longer have CRA intercepting API calls or CORS causing issues.
ghost
added
9 Ready to Merge
and removed
9 Waiting for Jenkins
9 Review Required
labels
|
Only unrelated errors in CI. |
5 participants
Scope & Purpose
This PR updates the frontend dependencies and replaces the ejected version of
create-react-appwithreact-app-rewired.This should get rid of the GitHub/npm audit nag spam.
It also replaces the yarn lockfile with the npm lockfile so frontend scripts should be invoked usingnpm runinstead ofyarnfrom now on. Note thatnpm installrequires the--legacy-peer-depsargument at the moment.This should also fix the weird behavior we were seeing when working with the devel version like nested frontends.
Checklist
Related Information