Skip to content

Conversation

danrosenthal
Copy link

@danrosenthal danrosenthal commented Jan 16, 2019

Turns out there is a fork of Card in web that relied on those contextTypes being exported, and updates to use new React context should really be considered a breaking change. This reverts all of #786 to avoid breaking changes.

See https://buildkite.com/shopify/web-ci-builder/builds/39489 and https://github.com/Shopify/web/pull/9824 for more context.

@danrosenthal danrosenthal changed the title re-export contentContextTypes as they were used in a fork of card in … re-export contentContextTypes as they were used in a fork of card in a consuming project Jan 16, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-884 January 16, 2019 18:59 Inactive
@danrosenthal danrosenthal added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Jan 16, 2019
Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Not tested but looks sensible.

Might be worth adding a comment above the export to say it's just for backwards compat purposes

@danrosenthal danrosenthal force-pushed the reexport-content-context-types branch from f1b94a9 to 8b9c7e4 Compare January 16, 2019 19:04
@BPScott BPScott temporarily deployed to polaris-react-pr-884 January 16, 2019 19:04 Inactive
Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

it passed build-consumer

@danrosenthal danrosenthal force-pushed the reexport-content-context-types branch from 8b9c7e4 to b4bc27b Compare January 16, 2019 19:14
@BPScott BPScott temporarily deployed to polaris-react-pr-884 January 16, 2019 19:15 Inactive
@danrosenthal danrosenthal force-pushed the reexport-content-context-types branch from b4bc27b to c179989 Compare January 16, 2019 19:18
@BPScott BPScott temporarily deployed to polaris-react-pr-884 January 16, 2019 19:18 Inactive
@danrosenthal danrosenthal force-pushed the reexport-content-context-types branch from c179989 to bfb3be5 Compare January 16, 2019 19:20
@BPScott BPScott temporarily deployed to polaris-react-pr-884 January 16, 2019 19:20 Inactive
@danrosenthal danrosenthal force-pushed the reexport-content-context-types branch from bfb3be5 to 32d0c4f Compare January 16, 2019 19:20
@BPScott BPScott temporarily deployed to polaris-react-pr-884 January 16, 2019 19:20 Inactive
@danrosenthal danrosenthal removed the 🤖Skip Changelog Causes CI to ignore changelog update check. label Jan 16, 2019
@danrosenthal danrosenthal changed the title re-export contentContextTypes as they were used in a fork of card in a consuming project revert [Banner] [Card] [Modal] convert banner legacy context API to use create context #786 Jan 16, 2019
Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Approved but sad that the whole thing can't be updated

@danrosenthal danrosenthal merged commit 8071acc into master Jan 16, 2019
@danrosenthal danrosenthal deleted the reexport-content-context-types branch January 16, 2019 19:41
@danrosenthal danrosenthal temporarily deployed to production January 16, 2019 19:49 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants