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

Update redux, react-redux and corresponding types #22442

Merged

Conversation

weltenwort
Copy link
Member

This updates redux to 4.0.0, which ships with built-in TypeScript typings. This allows to update the react-redux package to 5.0.7 as well in order to resolve typing conflicts between Kibana and x-pack.

Let me know if the changes break any conventions established around redux and TypeScript in Kibana core.

@weltenwort
Copy link
Member Author

I tagged this with :Sharing, because that seems to be the only Redux+TypeScript code I could find that required changes.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@stacey-gammon
Copy link
Contributor

cc @azasypkin

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

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

LGTM. Pulled and tested briefly but relying on ci to catch any nuanced redux changes that may break current implementations.

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM

"react-router-dom": "4.2.2",
"react-sizeme": "^2.3.6",
"react-toggle": "4.0.2",
"reactcss": "1.2.3",
"redux": "3.7.2",
"redux": "4.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

optional nit: we try to use "caret" versions for all dependencies we add and all dependencies we update since we started to use yarn.lock. But I see that it's used without caret in x-pack, so feel free to keep it as is, or update to "caret" versions in both package.json files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants