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

[v4] Update React - preparations #1392

Merged
merged 16 commits into from May 15, 2019
Merged

[v4] Update React - preparations #1392

merged 16 commits into from May 15, 2019

Conversation

AndrewMusgrave
Copy link
Member

WHY are these changes introduced?

Unfortunately we cannot continue updating our react components to prepare for react 16.8.* until we update to 16.8.* 😅 Enzyme doesn't support the new context at our current version, a bump in the version require a bump in other packages require a bump in react

WHAT is this pull request doing?

Updating react / enzyme

@BPScott BPScott had a problem deploying to polaris-react-pr-1392 May 1, 2019 15:20 Failure
@@ -1,5 +0,0 @@
// https://github.com/Microsoft/TypeScript/issues/24599
declare module 'prop-types' {
Copy link
Member Author

Choose a reason for hiding this comment

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

This was causing namespace issues

import {polarisAppProviderContextTypes} from '../types';
import AppProvider from '../AppProvider';

describe('<AppProvider />', () => {
it('passes i18n and withComponent properties to context', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test will be updated and improved when we switch AppProvider to the new context API

@@ -312,18 +312,20 @@ describe('<Modal>', () => {
});

describe('with app bridge', () => {
let AppBridgeModalCreate: jest.SpyInstance;
Copy link
Member Author

Choose a reason for hiding this comment

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

The next few files are using jest API to spy on functions rather than overwrite the function with a spy

@BPScott BPScott temporarily deployed to polaris-react-pr-1392 May 1, 2019 15:27 Inactive
@AndrewMusgrave AndrewMusgrave added the 🤖Skip Changelog Causes CI to ignore changelog update check. label May 1, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1392 May 1, 2019 16:57 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1392 May 1, 2019 19:22 Inactive
Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

Nice work, I added some onChange for error logs in tests if you can have a look at those.

@BPScott BPScott temporarily deployed to polaris-react-pr-1392 May 6, 2019 15:11 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1392 May 7, 2019 18:02 Inactive
Add types to test until

Remove polaris context namespace

Remove frame namespace

Export createContext result

Small quaility change

Rename context wrapper

Update themeprovider namespace

Add type

Add theme context to util

Update type name for app provider context

Update test utility and revert tsconfig changee
Convert ScrollTo

Remove dropzone context from index

Convert navigation

Convert combobox

Convert withincontext

Update frame context

Update resourcelist context

Update themeprovider context

Update appprovider context

Touchups

changelog

Update appprovider context type

Rename file
@BPScott BPScott temporarily deployed to polaris-react-pr-1392 May 15, 2019 16:28 Inactive
@danrosenthal danrosenthal self-requested a review May 15, 2019 16:29
@AndrewMusgrave
Copy link
Member Author

@danrosenthal I was planning on merging this into v4 since it has a ✅and every feature merged into this was ✅. Let me know if I should wait for a review from yourself/Tim 😄

@danrosenthal
Copy link
Contributor

danrosenthal commented May 15, 2019

@danrosenthal I was planning on merging this into v4 since it has a ✅and every feature merged into this was ✅. Let me know if I should wait for a review from yourself/Tim 😄

No need, was just adding us there so we could follow along. :shipit:

@AndrewMusgrave AndrewMusgrave merged commit cb8a3f7 into version-4.0.0 May 15, 2019
@AndrewMusgrave AndrewMusgrave deleted the update-react branch May 15, 2019 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖Skip Changelog Causes CI to ignore changelog update check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants