[FOLIO-2433] in dev-mode, react has a memory leak Created: 23/Jan/20 Updated: 22/Oct/20 Resolved: 29/Apr/20 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Bug | Priority: | P3 |
| Reporter: | Zak Burke | Assignee: | Ryan Berger |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||||||||||
| Sprint: | stripes-force 85, stripes-force 86, stripes-force 87 | ||||||||||||||||||||||||||||
| Development Team: | Stripes Force | ||||||||||||||||||||||||||||
| Description |
|
We run CI tests with React in dev-mode, but React leaks event listeners in dev-mode. This is likely a contributing factor to the unit tests that randomly fail in ui-users. Our unit test interactors lean heavily on selectors for data-test-* attributes which are stripped out when we generate a bundle in production-mode. We should investigate whether it is possible to generate a bundle with React in production-mode but with stripes in dev-mode so we can keep those attributes. |
| Comments |
| Comment by Zak Burke [ 23/Jan/20 ] |
|
John Malconian, do you know if/how we can configure our builds when running unit tests in CI so that we keep the data-test-* attributes and use React in production mode? I don't know what step in our process drives the stripping of those attributes. IIRC React relies on NODE_ENV=production. |
| Comment by Zak Burke [ 11/Feb/20 ] |
|
WRT data-test-* attributes, it looks like those are stripped depending on the environment via instructions in the webpack.config.cli.$ENV.js file: babelLoaderConfig.options.plugins = (babelLoaderConfig.options.plugins || []).concat([ [require.resolve('babel-plugin-remove-jsx-attributes'), { patterns: ['^data-test.*$'] }] ]); If we create a webpack.config.cli.test.js config that puts React in production mode but leaves the data-test-* attributes alone and configure stripes-cli to use it I think we might be in good shape. |
| Comment by Zak Burke [ 11/Feb/20 ] |
|
Of course it isn't that easy. Tests fail when run like this: NODE_ENV=production yarn test We can set mode: 'production' in the webpack config, but it isn't clear to me how mode: 'production' and NODE_ENV='production' interact as the browser console fills with warnings, suggesting React is not, in fact, in production mode. |
| Comment by Ryan Berger [ 15/Apr/20 ] |
|
After running `ui-users` tests (v3.0.0 tag) in Jenkins with the new `--bundle` flag to force production mode the tests have stabilized significantly. I do think that even though this is helping memory, there are still issues to be corrected and as more tests are added we will eventually run into this problem again. I do recommend using this path to re-enable the tests and fix any tests in master that have broken since v3.0.0. But, we should also be looking at other ways to address memory leaks. One such suggestion came from Zak Burke who mentioned that in BigTest2 they will be running tests in frames so that the frame can be disposed of after test completion to allow for the browser to do garbage collection. We could consider looking into possibly adopting that in our current BigTest implementation. |
| Comment by Ryan Berger [ 22/Apr/20 ] |
|
Merged into ui-users. Master is now "mostly" stable. Certainly passing at a much higher rate than before. Closing this ticket as the primary objective has been completed and other memory investigations will be tracked separately. |