Updated Jest & Create React App #836
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this change, Test errors were output in too many places making tests hard to read This change Is work in progress to reolve test issues. Only some service packages have been fixed so far.
Prior to this change, Search tests had typescript issues in Jest 24 This change Fixes outstanding Typescript issues in the search package
Prior to this change, tests failed after upgrading to Jest 24 This change Fixes the user-mgnt package
Prior to this change, Jest-fetch-mock types were conflicting This change Took the opportunity to align jest-fetch-mock, node-fetch and @types/node-fetch versions across the packages
…e types must be overridden by any in tests. Also deleted no longer used validation module Prior to this change, Jest-fetch-mock types were broken everywher after jest upgrade This change Removes use of jest-fetch-mock types and removes a legacy, unused module - validation
Prior to this change, Jest 24 was incompatible entirely with react-scripts-ts This change Upgrades clients to create-react-app 3.0.1
Prior to this change, The baseUrl of services was '.' This change Aligns the services with the CRA approach for the clients setting baseUrl to './src' and making an import alias
Prior to this change, CRA v3.0 forbade absolute paths using an alias This change Patches react-scripts to all absolute paths as this is an open PR and will eventually become part of CRA. Change all pahts in register app to follow @register
Prior to this change, Linting is now built into CRA with eslint. TSlint will soon be deprecated This change Is work in progress. Have added typescript-eslint dependencies. Still testing
Prior to this change, I had attempted to patch CRA to resolve aliases but it didnt work This change Uses craco instead and eslint and jest appears to be working in register app
Prior to this change, Upgrade to ESlint caused many typescript issues This change Fixes many of them, particularly in validations
Prior to this change, Not all previous typescript issues were visible in Eslint This change fixes ones that became visible in VSCode after previous set were resolved.
Prior to this change, Login and performance app had no alias and contained relative paths This change Adds craco and amends all imports in login and performance
Prior to this change, Import plugin was not successfully added and I need to test relative import warning This change Correctly adds the plugin and yarn lint:ts now functions
Prior to this change, Some rules were enabled that we do not need to support This change Ignores those rules that we do not need
Prior to this change, Many faults stopped the app from building This change Resolves 30 issues and upgrades redux-loop
Prior to this change, Register wasnt compiling This change Makes sure register app can compile
Prior to this change, Login app was not compiling This change Fixes remaining typescript issues so that the login app can compile
Prior to this change, Performance app was not compiling This change Fixes remaining Typescript errors and performance app compiles
Prior to this change, The correct plugin had not been added correctly into the eslintrc.js This change Adds in the correct plugin so that the import rule works
…e added to resolve fragmentMatching bug in MockedProvider Prior to this change, There was no record of these chages This change Adds a TODO comment to remove these objects when fragmentMatching bug is resolved.
This was referenced Jun 11, 2019
Fixes tests after merge and moves location of tslint script in services
Prior to this change, The certificate jounrey requires updating and the tests cant be resolved This change deletes the single test for this file
… pinned hapi-jwt-auth Prior to this change, The tests failed and hapi-jwt-auth version was incompatible This change Comments out failing test as it needs re-written
Prior to this change, Builds would not complete with warnings This change Patches react-scripts to remove throwing this error
rcrichton
reviewed
Jun 14, 2019
| @@ -3,7 +3,7 @@ const { styles, theme } = require('./styleguide.styles') | |||
| const path = require('path') | |||
|
|
|||
| function getWebpackConfig() { | |||
| const webpackConfig = require('react-scripts-ts/config/webpack.config.dev.js') | |||
| const webpackConfig = require('./config/webpack.config.dev.js') | |||
There was a problem hiding this comment.
If we are only using the dev config do we still need all the other config files.
There was a problem hiding this comment.
No, but if we ever want to amend the config then it might be easier to have these scripts. For now I think we should keep them as they are generated by CRA eject for that purpose.
| @@ -199,9 +201,12 @@ class HeaderComp extends React.Component<IProps, IState> { | |||
| const { userDetails, language, intl } = this.props | |||
|
|
|||
| let name = '' | |||
| if (userDetails && userDetails.name) { | |||
| if (userDetails && userDetails.name && userDetails.name) { | |||
There was a problem hiding this comment.
Isn't this condition repeated.
There was a problem hiding this comment.
Thanks, removing.
| testFhirBundle.entry[1].resource.identifier && | ||
| testFhirBundle.entry[1].resource.identifier[1] && | ||
| testFhirBundle.entry[1].resource.identifier[1].value | ||
| ) { |
There was a problem hiding this comment.
The one issue with all these if statements that have been added is if one of these if statements return false then the tests still pass, it would be better to fail the test if the if statement return false.
There was a problem hiding this comment.
Added fail cases: 292e6b6
mushrafulhoque-dsi
approved these changes
Jun 17, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses some of the dependencies that are required in order to speed up Typescript builds in Docker and also provide some general improvements to code style and provide Typescript linting support in tests.
yarn start:prodis called. Craco has no way to disable this warning as it happens outside of webpack.