Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Fix peer dependencies #387

Closed
wants to merge 7 commits into from
Closed

Fix peer dependencies #387

wants to merge 7 commits into from

Conversation

nickmccurdy
Copy link

@nickmccurdy nickmccurdy commented Sep 1, 2018 •
edited

Fixes #373.

Continued from #376, merged with master, and tested to fix all peer dependency warnings.

This was referenced Sep 1, 2018
Closed
Open
@wmonk
Copy link
Owner

wmonk commented Sep 2, 2018

@nickmccurdy I messed up the deps on master, so if you could rebase then that should fix failing CI!

@wmonk wmonk mentioned this pull request Sep 2, 2018
Closed
@nickmccurdy
Copy link
Author

I fixed the package's own peer dependencies, but it seems like there are still unmet peer dependencies in the e2e tests. Let's see if I can fix them first.

@nickmccurdy
Copy link
Author

nickmccurdy commented Sep 3, 2018 •
edited

The other errors seem to be false positives. This should be ready to merge. Sorry I couldn't figure out how to rebase these together into one commit without having to resolve the same conflicts again, could you squash merge (should be equivalent)?

@martijnthe
Copy link

Does this fix the warnings that yarn is giving me after creating a freshly generated react typescript app?

warning "workspace-aggregator-a8fb5286-b8b7-4413-ab03-0a73a4550814 > my-app > react-scripts-ts@2.17.0" has incorrect peer dependency "typescript@2.x".
warning "workspace-aggregator-a8fb5286-b8b7-4413-ab03-0a73a4550814 > my-app > react-scripts-ts > fork-ts-checker-webpack-plugin@0.2.10" has incorrect peer dependency "typescript@^2.1.0".
warning "workspace-aggregator-a8fb5286-b8b7-4413-ab03-0a73a4550814 > my-app > react-scripts-ts > ts-jest@22.0.1" has incorrect peer dependency "typescript@2.x".

@nickmccurdy
Copy link
Author

Yes, it should.

@Fabianopb
Copy link

Nice, Jest 23 coming along. CI seems happy, any plans when this will be released? In 2.18.0 perhaps?

r3nya
r3nya reviewed Sep 28, 2018
View reviewed changes
config/jest/typescriptTransform.js
@@ -2,6 +2,6 @@

'use strict';

const tsJestPreprocessor = require('ts-jest/preprocessor');
const tsJestPreprocessor = require('ts-jest');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can remove this file and add

"transform": {
   "^.+\\.tsx?$": "ts-jest",
}

into scripts/utils/createJestConfig.js config.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to be able to remove this workaround, but it should be tested thoroughly.
There were some issues in the past regarding the proper resolution in both ejected and non-ejected environments. This wrapper file was a (kinda hacky, but working) way to get around these issues.

r3nya
r3nya reviewed Sep 28, 2018
View reviewed changes
Copy link
Contributor

@r3nya r3nya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update jest and ts-jest as well.

package.json
"case-sensitive-paths-webpack-plugin": "2.1.1",
"chalk": "1.1.3",
"css-loader": "0.28.7",
"dotenv": "4.0.0",
"dotenv-expand": "4.2.0",
"extract-text-webpack-plugin": "3.0.2",
"file-loader": "1.1.5",
"fork-ts-checker-webpack-plugin": "^0.2.8",
"fork-ts-checker-webpack-plugin": "0.4.4",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest version is 0.4.9.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On master this dependency version was upgraded and then downgraded back to 0.2.x, any idea why?

@nickmccurdy
Copy link
Author

nickmccurdy commented Oct 21, 2018 •
edited

I'm closing this now that facebook/create-react-app#4837 is merged into CRA.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@insidewhy insidewhy insidewhy left review comments

@r3nya r3nya r3nya left review comments

@DorianGrey DorianGrey DorianGrey left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix peer dependencies for TS 3
7 participants
@nickmccurdy @wmonk @martijnthe @Fabianopb @insidewhy @r3nya @DorianGrey