Project

General

Profile

Actions

Task #1846

closed

Upgrade the frontend

Added by Vít Švanda over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Vít Švanda
Category:
Frontend
Target version:
Start date:
09/30/2019
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Owner:

Subtasks 2 (0 open2 closed)

Task #1882: Dynamic generating a <Route> from routes.jsClosedVít Švanda09/30/2019

Actions
Task #1915: Replace a UNSAFE_componentWillReceiveProps(nextProps)ClosedVít Švanda10/29/2019

Actions

Related issues

Related to IdStory Identity Manager - Task #1801: Upgrade backend dependenciesClosedRadek Tomiška08/16/2019

Actions
Actions #1

Updated by Vít Švanda over 4 years ago

I use official conversion utility for move PropTypes. I had problem with out of memory, so I had to run command for every module separately:

npx react-codemod React-PropTypes-to-prop-types

componentWillReceiveProps were renamed to UNSAFE_componentWillReceiveProps

npx react-codemod rename-unsafe-lifecycles --force

TODO: https://github.com/bcvsolutions/CzechIdMng/commit/00d7704acd1a0f3acf664791ef1d6c6189d29839


Replaces:

import TestUtils from 'react-addons-test-utils'; -> import TestUtils from 'react-dom/test-utils';

react-router-redux is death -> replace with connected-react-router


We need to upgrade the react-router to version 4.
- Migration https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/guides/migrating.md#patternutils

https://codeburst.io/react-router-v4-unofficial-migration-guide-5a370b8905a


React-redux replace:

withRef: true -> forwardRef: true

Tests use Shallow renderer from the react-test-renderer:

Replace:

 const shallowRenderer = TestUtils.createRenderer(); -> const shallowRenderer = new ShallowRenderer();

To modified files add import ShallowRenderer from 'react-test-renderer/shallow';.


TODO: AbstractComponent-test was disabled !!!


Replace:

import { Link } from 'react-router'; -> import { Link } from 'react-router-dom';

Redux context is no longer in this.context.store. If you use the state directly (this.context.store.getState()), you have to rewrite logic for use react-redux select method in connect.


Replace:
Redux context is no longer in this.context.store. Dispatch method is in the props.

Replace is not necessary, I implemented ours IdmContext with redux store!

this.context.store.dispatch( -> this.props.dispatch(


React component doesn't have no longer instance of owner. I had to modified AbstractForm for use workaround (not nice, but it works).


Replace:
React router is in IdmContext only. For replace/push/goBack .. use this.context.history.

this.context.router -> this.context.history


Replace:
Method getWrappedInstance() on a redux component no longer exists, because is no necessary (if { forwardRef: true } is used).

.getWrappedInstance(). -> .

Url parameters are no longer in props.params. Use props.match.params instead.

Replace:

component.params -> component.match.params

Replace:

this.props.params -> this.props.match.params

Replace:

params={ this.props.match.params } -> match={ this.props.match }

params={this.props.match.params} -> match={ this.props.match }

Replace:

props.params -> props.match.params

nextProps.params -> nextProps.match.params

Manual replace:

ONLY for component using Advanced.TabPanel replace this.props.children -> this.getRoutes()

If you using redirect via replace or push method, then you have to ensure correct format of the path. Previouse version of react-router used absolute path only. This is not true now!

So, if you have this redirection:

this.context.history.replace(`systems`);

, then final url will be http://localhost:3000/#/currentUrl/systems!

If you want to use absolute path http://localhost:3000/#/systems, you have to use in this case:

this.context.history.replace(`/systems`);!

Suggestion is try to search (this.context.history.push(`, this.context.history.replace(`) and check if all useing of replace/push starts with slash. Replace all is not good idea, because in some cases can starts expression with call a method.


Same situation as previous occurs for <Basic.LinkCell> and <Advanced.ColumnLink> components, where attribute to is no longer absolute. So you need to add slash (if missing on the start of path).

Suggestion is try to search (to=") and check if all useing starts with slash. Replace all is not good idea, because in some cases can starts expression with call a method.

Example: <Basic.LinkCell property="key" to="workflow/definitions/:key"/> -> <Basic.LinkCell property="key" to="/workflow/definitions/:key"/>


React router since V4 doesn't parse the query parameters (.../?new=1) :-(.

I implemented parsing in AbstractContextComponent._parseUrlQuery.
Parsing is executed in constructor and result query object is sets to the this.props.location.query (for ensure back compatibility).

Beware this.props.location.query could be null, because redux calling Select method before constructor of AbstractContextComponent. So you have to prevent of throwing of the null pointer, typicaly in select mothod (for example: component.location ? component.location.query.automaticRoleId should be modified on component.location && component.location.query ? component.location.query.automaticRoleId : null;).


Method checkAccess in the SecurityManager was removed. This logic was moved to the AbstractContextComponent._getComponent(route).


Modal component from react-bootstrap contains bug! Modal dialog is show before subcomponents are rendered. So if you call setState for show a modal dialog and in callback you want to set focus on some component (this.refs.firstName.focus()), then you will obtain exception, because the ref for that component will be null.

I found ugly but working solution, it is the timeout (10ms). This timeout is implemented in AbstractTableContent.showDetail. It means, if you use this method, your modal dialog should be correctly rendered. If you showing modal dialog manually, then you have to add timeout or use method showDialog.

Here is example:

Not works:

  this.setState({
      detail: {
        show: true,
        entity
      }
    }, () => {
      this.refs.generatorType.focus();
    });

Works:

  super.showDetail(entity, () => {
      this.refs.generatorType.focus();
    });


Actions #2

Updated by Vít Švanda over 4 years ago

I tried to update dependencies for backend of the frontend. It means gulp, babel, browsersync. I slolved many issues, but I ended on serviring a files by browserify. That operation never end (witout any error). I think the problem is in gulp vs babel.

I reveted my changes and only what I updated is browserify and browser-sync dependencies. It works, but only one issue remains ... automatic reload of the browsers does not work now.

Actions #3

Updated by Vít Švanda over 4 years ago

I had big problem with Moda dialog. After upgrade on React 16 has bug, occurred when we need to access to the rendered components (via refs). Typicaly in the situation, when we need to set focus to the component or set data to the form.

I had to use the timeout (so now it works, but it is ugly), because Modal doesn't have rendered refs in this phase.
This problem occured after update on React 16, but primary bug is in react-bootstap.
Problem should be fixed, but still doesn't works (in 0.32.4).
https://github.com/react-bootstrap/react-bootstrap/issues/2841#issuecomment-378017284.

Actions #4

Updated by Vít Švanda over 4 years ago

React router since V4 doesn't parse the query parameters (.../?new=1) :-(.

I implemented parsing in AbstractContextComponent._parseUrlQuery.
Parsing is executed in constructor and result query object is sets to the this.props.location.query (for ensure back compatibility).

Actions #5

Updated by Vít Švanda over 4 years ago

I dicovered big issue. If data are set to the Form, then in setState callback aren't data correctly sets in form components!
I solved problem with no selection of the navigation.

Actions #6

Updated by Vít Švanda over 4 years ago

I found solution/workaround for AbstractForm. Problem is in method setData, where are data set to every components of this form. Set state in components (setValue) are stated asynchronly, so AbstractForm, doesn't have current data after his own setState method ends.

Solution is: Before setValue for every component is called I sets same value direct to the component state (component.state.value = value). In this case has AbstractForm data immediately.

Actions #7

Updated by Vít Švanda over 4 years ago

Actions #8

Updated by Vít Švanda over 4 years ago

I found problem with react-boostrap Popover. Content showing in the popover is probablly rendered as new instance of React. This caused problem with Redux and Router, because that rendered content doesn't contains a Redux state and Router context.

So I had to wrapp the content to the new Redux provider and Router.

Because I need to propagate the redux store, I had to made ours Basic.Popover context able ... so extends from AbstractContextComponent now!

This workaround works correctly for now.

Actions #9

Updated by Vít Švanda over 4 years ago

I found a problem in AbstractFrom, when some of form component is wrapped as Redux component, then AbstractForm doesn't found this component. Not worked for TreeNodeSelect and PasswordChangedComponent ...

I implemented additional search of Redux component to the AbstractForm. If Redux component is found, then I extracts original AbstractFromComponent.
This working correctly for now.

Actions #10

Updated by Vít Švanda over 4 years ago

Actions #11

Updated by Vít Švanda over 4 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Vít Švanda to Radek Tomiška
Actions #12

Updated by Radek Tomiška over 4 years ago

  • Related to Task #1801: Upgrade backend dependencies added
Actions #14

Updated by Radek Tomiška over 4 years ago

I tested the whole FE and fixed some issues. Except some styles and minor issues, even fixed reports from other modules:
https://github.com/bcvsolutions/CzechIdMng/commit/22c18d9eb8435d28512d7cc6e734bddfd14bf136

Remaining issues:
- ✔️ role mapping overload - blank page (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/b1eee090b553106ac20aab633afcd4d0960267a3)
- ✔️ the role of system mapping cannot be deleted from the system side (maybe it is good, but on the other hand there is a shine add button ... one or the other is ok) (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/ffb22ff6cfabd37b13000fcf64d4b683055b187e)
- ✔️ virtual request approval from request detail (works on dashboard) - this.context.router is undefined (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/b1eee090b553106ac20aab633afcd4d0960267a3)
- ✔️ Requests for change roles (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/5ad8b5a711ac6843caeb892dd3b449dca0756633)
- ✔️ Audit - Automatic roles - Modal of rule - Is not readonly (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/b1eee090b553106ac20aab633afcd4d0960267a3)
- ✔️ Notification template save - blank page (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/b1eee090b553106ac20aab633afcd4d0960267a3)
- ✔️ notification template backup + redeploy (scripts to same) => file name is made from user usernam, which can contain crap (see encoding issue, but also on BE). (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/841c22a9532d6b54c89f1f4a4cdfed071696a036)
- ✔️ scheduled task - add trigger not work (setTimeout is missing) (Commit: https://github.com/bcvsolutions/CzechIdMng/commit/b1eee090b553106ac20aab633afcd4d0960267a3)

Actions #15

Updated by Vít Švanda over 4 years ago

  • Assignee changed from Radek Tomiška to Vít Švanda
Actions #16

Updated by Vít Švanda over 4 years ago

  • Status changed from Needs feedback to In Progress
Actions #17

Updated by Radek Tomiška over 4 years ago

  • Status changed from In Progress to Resolved

I did test and code review for fixes above, all issues are fixed, thx!

Actions #18

Updated by Vít Švanda over 4 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF