Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migration to react-navigation v2.x (#3068) #3141

Merged
merged 20 commits into from
Aug 8, 2018
Merged

Conversation

aksonov
Copy link
Owner

@aksonov aksonov commented Jul 22, 2018

@daviscabral
Copy link
Collaborator

@aksonov I've brought your list here - and placed my username beside the item I am planning to tackle first.

@@ -12,16 +12,16 @@
"mobx": "^3.2.1",
"mobx-react": "^4.2.1",
"prop-types": "^15.5.10",
"react": "16.4.1",
"react-native": "0.56.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to revert this here? Something that I might help? I was thinking about using the last version for everything. I've spent a couple of days in the past week just to make a few projects to work with these versions.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, please try to fix. I've got some babel-related error, connected with stage 0 or something like that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool - I think I know how to fix them. This might be related to the use of babel 7 (I had to upgrade a few other deps in order to make it to work - checking it).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got example working with latest react and react-native.

ezgif-5-da09ec92f0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also from:

✖ 538 problems (535 errors, 3 warnings)

To:

✖ 136 problems (133 errors, 3 warnings)

daviscabral and others added 5 commits July 24, 2018 09:14
…#3142)

* Link to docs and example app in README (#3135)

* Babel v7, ESlint fixes, React/React Navigation upgrades, mobx removed

* Test circleci config

* Typo

* Bump node version

* Fix missing dep

* Test fixes

* Wrong deps

* Update lockfiles

* Update lockfiles

* get rid of dist

* Tab updates and more lint fixes

* Test fix

* Fix navigatonStore.reset and more lint errors

* Warning for those rules while we are still moving things
@daviscabral
Copy link
Collaborator

I believe you are using the plugin for prettier only - there is a better plugin with support to eslint + prettier on VSCode - better to use that one instead.

@aksonov
Copy link
Owner Author

aksonov commented Jul 24, 2018

Could you say name of plugin? There is a lot..

@daviscabral
Copy link
Collaborator

@aksonov this one:

https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode

You have to enable the ESlint integration:

prettier.eslintIntegration: true

Also, instead of prettier, use prettier-eslint, so it will apply it in the right order.

@daviscabral
Copy link
Collaborator

👏 👏 👏 👏

@aksonov
Copy link
Owner Author

aksonov commented Jul 24, 2018

@daviscabral I'm done for today. If you have time please fix custom actions, as well replace action, also I found missed labels for two tabs, etc. Btw, I introduced easier custom navigator implementation now - 'renderers', i.e. views for custom navigators, please check OverlayRenderer and LightboxRenderer.

@daviscabral
Copy link
Collaborator

@aksonov sure - the custom actions are almost done. The missing labels are the next thing to do.

@aksonov
Copy link
Owner Author

aksonov commented Jul 24, 2018

Oh, also custom onNavigationStateChange handler.

@daviscabral
Copy link
Collaborator

Got busy with work and couldn't finish what I've started - but planning tackle that later 👍

@@ -471,25 +466,69 @@ function uniteParams(routeName, params) {
return res;
}

const defaultSuccess = () => {};
const defaultFailure = () => {};

Copy link
Collaborator

Choose a reason for hiding this comment

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

My bad here for removing it - they were not being used before.

Copy link
Owner Author

Choose a reason for hiding this comment

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

No worries, you just fixed eslint errors. It was used by removed setState, now I re-enabled handlers.

@aksonov
Copy link
Owner Author

aksonov commented Jul 25, 2018

@daviscabral Please check deep linking if you can, it may not work now..

@aksonov
Copy link
Owner Author

aksonov commented Jul 26, 2018

@daviscabral any ETA for REPLACE action?

@daviscabral
Copy link
Collaborator

Checking it now - back with ETA in a few.

* Navigation and replace restored
@daviscabral
Copy link
Collaborator

I believe there are still more things besides what is listed in the PR todo list - do you recall anything else @aksonov?

@aksonov
Copy link
Owner Author

aksonov commented Jul 26, 2018 via email

@daviscabral
Copy link
Collaborator

daviscabral commented Jul 26, 2018 via email

@aksonov
Copy link
Owner Author

aksonov commented Jul 26, 2018 via email

@daviscabral
Copy link
Collaborator

Checking.

@daviscabral
Copy link
Collaborator

@aksonov can you elaborate on this to me?

            // go to first screen of the StackNavigator with reset
            // navigation.dispatch(NavigationActions.reset({
            //   index: 0,
            //   actions: [NavigationActions.navigate({ routeName: tab.route.routes[0].routeName })],
            // }));
            // go to first screen of the StackNavigator without reset
            for (let i = 1; i < scene.route.routes.length; i += 1) {
              navigation.dispatch(NavigationActions.back());
            }

Why are we using multiple back calls instead of just one single reset there?

@aksonov
Copy link
Owner Author

aksonov commented Jul 27, 2018

@daviscabral Because reset was buggy and didn't work for sub-tree, feel free to remove it

@aksonov aksonov changed the title WIP: migration to react-navigation v2.x (#3068) migration to react-navigation v2.x (#3068) Aug 8, 2018
@aksonov aksonov merged commit d61960c into master Aug 8, 2018
@daviscabral daviscabral deleted the react-navigation-v2 branch August 8, 2018 10:52
@daviscabral daviscabral added the backwards-incompatible Breaking changes label Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatible Breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants