Fix test --coverage errors related to TS #8

horacioh · 2018-10-17T22:58:26Z

also I took out all the pre-commit/format config from this branch. I think the best way is to fix this in another PR.

horacioh · 2018-10-17T23:14:01Z

I hate when things in my machine works and in others doesn't... :(

…tion-core into feature/typescript Conflicts: package.json yarn.lock

horacioh · 2018-10-18T00:15:12Z

ok I need a little help here. not getting too much progress :(

it is strange that in the circleci log we are getting the typescript help text and not locally?

if someone with more experience with circleci can help that would be great!. I'm sure is a really silly (and funny) error...

horacioh · 2018-10-18T00:18:22Z

i noticed this: https://github.com/react-navigation/react-navigation-core/blob/master/.circleci/config.yml#L5

maybe using node 7 is causing any problem? I'm using 9.4.0 locally..

package.json

  },
  "peerDependencies": {
    "react": "*"
  },
  "jest": {
    "preset": "react-native",
-    "testRegex": "/__tests__/[^/]+-test\\.js$",
+    "testRegex": "/__tests__/[^/]+-test\\.[jt]sx?$",


package.json

+        "tsConfig": "./tsconfig.test.json",
+        "diagnostics": {
+          "ignoreCodes": [
+            151001


.gitignore

+
+# TypeScript-generated files
+*.d.ts
+!src/react-navigation.d.ts


horacioh · 2018-10-22T09:15:42Z

ok everything solved :)
the problem was that the tsconfig.json was listed in the .gitignore. I knew it was a silly error...

package.json

tsconfig.json

.gitignore

+
+# TypeScript-generated files
+*.d.ts
+!src/react-navigation.d.ts


Conflicts: example/App.js example/src/SimpleStack.js src/getChildEventSubscriber.ts

horacioh · 2018-10-24T01:01:41Z

Finally, because of some updates to getChildEventSubscriber I needed to roll back to the JS version for now. there are some other files in TS and everything now it seams to work.

sorry for the mess with this PR, I need to properly setup my machine to be more reliable with the CI, a lot of trials and errors. or maybe I can learn a better way to maintain this contributions cleaner?

hope this now can be approved :)

brentvatne · 2018-10-25T20:44:24Z

ok let's try this again :) thanks @horacioh and @satya164!

satya164 · 2018-10-25T20:58:03Z

@horacioh you forgot to fix this buggy regex: #8 (comment)

brentvatne · 2018-10-25T21:20:09Z

@horacioh - can you open a followup pr for that? thx 🙏

brentvatne · 2018-10-26T15:10:43Z

I can't get the example project running with typescript :( here's my attempt: https://github.com/react-navigation/react-navigation-core/tree/%40brent/attempted-example-fix-ts

horacioh · 2018-10-26T17:17:32Z

checking it now @brentvatne

horacioh · 2018-10-26T18:43:42Z

@brentvatne I got the example project working in your branch:
it appears that the dependencies we have listed in the react-navigation-core package.json file are not being installed in the example project. the two packages that are required and not installed in the example are path-to-regexp and query-string.

let me dig dipper into why this two packages are not installed as a dependency in the example project.

maybe because the @react-navigation/core package is marked as devDependency inside the @react-navigation/native package?

This reverts commit 27ed333, reversing changes made to a4da6ff. # Conflicts: # package.json

ericvicenti · 2018-10-30T01:40:27Z

Sorry I had to revert this change because I needed to get examples running again: 1897b32

maybe because the @react-navigation/core package is marked as devDependency inside the @react-navigation/native package?

Let me know if this is the problem and I can make a release of /native without it. Its funny how our real devDependencies seem to be the dependencies of the example project

horacioh · 2018-10-31T18:11:07Z

I think this is the problem. I manually add it as a dependency and it was working correctly.

so if you add @react-navigation/core as a dependency, this installs the package properly in the example project and works.

hope this helps :)

horacioh · 2018-10-31T18:16:19Z

@ericvicenti do you want me to do a PR of this?

brentvatne · 2018-10-31T19:38:39Z

@horacioh - yeah that would be great! thanks

horacioh · 2018-11-02T11:14:21Z

is the example working in master? its not for me. I tried to link the local packages to test my changes before pushing any change and now i got other errors :(

trying to resolve this issues before submitting another PR. sorry!

brentvatne · 2018-11-02T17:27:14Z

hey @horacioh - it's running for me on master.

horacioh · 2018-11-10T17:14:56Z

hey @brentvatne !
could you please check this? I think I got some progress on the TS migration.

cc @satya164 @ericvicenti

Horacio Herrera added 2 commits October 18, 2018 00:50

fix ts test errors + remove formatters

edbd479

add husky again

3d5836e

Horacio Herrera added 6 commits October 18, 2018 01:20

fix ts test errors + remove formatters

75aa6a0

add husky again

22236bb

Merge branch 'feature/typescript' of github.com:horacioh/react-naviga…

092c037

…tion-core into feature/typescript Conflicts: package.json yarn.lock

yarn install update

833c07b

change new ts file for a simpler one

383b713

add another ts file

80a8a0f

Merge branch 'master' into feature/typescript

4baa953

satya164 reviewed Oct 21, 2018

View reviewed changes

package.json

"tsConfig": "./tsconfig.test.json",

"diagnostics": {

"ignoreCodes": [

151001

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

satya164 reviewed Oct 21, 2018

View reviewed changes

add tsconfig to git (it was ignored)

1665c14

satya164 reviewed Oct 22, 2018

View reviewed changes

package.json Show resolved Hide resolved

tsconfig.json Outdated Show resolved Hide resolved

.gitignore Outdated

# TypeScript-generated files

*.d.ts

!src/react-navigation.d.ts

This comment was marked as abuse.

Sign in to view

Horacio Herrera and others added 6 commits October 22, 2018 23:53

update gitignore

8392a50

remove tsconfig.json comments

ce562bf

Merge branch 'master' into feature/typescript

5bdc868

fix merge with master

ef2ee0c

Merge branch 'master' into feature/typescript

449d080

Conflicts: example/App.js example/src/SimpleStack.js src/getChildEventSubscriber.ts

remove getChildEventSubscriber.ts file bc of conflicts with updates

f67807b

brentvatne approved these changes Oct 25, 2018

View reviewed changes

brentvatne merged commit 27ed333 into react-navigation:master Oct 25, 2018

horacioh deleted the feature/typescript branch October 25, 2018 23:56

ericvicenti added a commit that referenced this pull request Oct 30, 2018

Revert "Merge pull request #8 from horacioh/feature/typescript"

1897b32

This reverts commit 27ed333, reversing changes made to a4da6ff. # Conflicts: # package.json

horacioh mentioned this pull request Feb 25, 2020

Fix error in testRegex config react-navigation/react-navigation#7651

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix test --coverage errors related to TS #8

Fix test --coverage errors related to TS #8

horacioh commented Oct 17, 2018

horacioh commented Oct 17, 2018

horacioh commented Oct 18, 2018

horacioh commented Oct 18, 2018

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

horacioh commented Oct 22, 2018

This comment was marked as abuse.

horacioh commented Oct 24, 2018

brentvatne commented Oct 25, 2018

satya164 commented Oct 25, 2018

brentvatne commented Oct 25, 2018

brentvatne commented Oct 26, 2018

horacioh commented Oct 26, 2018

horacioh commented Oct 26, 2018 •

edited

Loading

ericvicenti commented Oct 30, 2018 •

edited

Loading

horacioh commented Oct 31, 2018 •

edited

Loading

horacioh commented Oct 31, 2018

brentvatne commented Oct 31, 2018

horacioh commented Nov 2, 2018

brentvatne commented Nov 2, 2018

horacioh commented Nov 10, 2018 •

edited

Loading

Fix test --coverage errors related to TS #8

Fix test --coverage errors related to TS #8

Conversation

horacioh commented Oct 17, 2018

horacioh commented Oct 17, 2018

horacioh commented Oct 18, 2018

horacioh commented Oct 18, 2018

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

horacioh commented Oct 22, 2018

This comment was marked as abuse.

horacioh commented Oct 24, 2018

brentvatne commented Oct 25, 2018

satya164 commented Oct 25, 2018

brentvatne commented Oct 25, 2018

brentvatne commented Oct 26, 2018

horacioh commented Oct 26, 2018

horacioh commented Oct 26, 2018 • edited Loading

ericvicenti commented Oct 30, 2018 • edited Loading

horacioh commented Oct 31, 2018 • edited Loading

horacioh commented Oct 31, 2018

brentvatne commented Oct 31, 2018

horacioh commented Nov 2, 2018

brentvatne commented Nov 2, 2018

horacioh commented Nov 10, 2018 • edited Loading

horacioh commented Oct 26, 2018 •

edited

Loading

ericvicenti commented Oct 30, 2018 •

edited

Loading

horacioh commented Oct 31, 2018 •

edited

Loading

horacioh commented Nov 10, 2018 •

edited

Loading