Fix cyclical type imports relating to color-picker helpers #767
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BPScott
force-pushed
the
cyclical-imports-THEEND
branch
from
34c8de7 to
8cb2d62
Compare
tmlayton
reviewed
Dec 17, 2018
UNRELEASED.md
Outdated
|
|
||
| - Added a slight delay to the Percy screenshot script to give time for components to render fully ([#704](https://github.com/Shopify/polaris-react/pull/704)) | ||
| - Refactors to remove cyclical type imports ([#759](https://github.com/Shopify/polaris-react/pull/759) and [#754](https://github.com/Shopify/polaris-react/pull/754)) | ||
| - Refactors to remove cyclical type imports ([#759](https://github.com/Shopify/polaris-react/pull/759), [#754](https://github.com/Shopify/polaris-react/pull/754) and [#767](https://github.com/Shopify/polaris-react/pull/767)) |
There was a problem hiding this comment.
Suggested change
| - Refactors to remove cyclical type imports ([#759](https://github.com/Shopify/polaris-react/pull/759), [#754](https://github.com/Shopify/polaris-react/pull/754) and [#767](https://github.com/Shopify/polaris-react/pull/767)) | |
| - Refactors to remove cyclical type imports ([#759](https://github.com/Shopify/polaris-react/pull/759), [#754](https://github.com/Shopify/polaris-react/pull/754), and [#767](https://github.com/Shopify/polaris-react/pull/767)) |
Oxford comma
There was a problem hiding this comment.
The strippers, JFK, and Stalin appreciate your attention to detail.
(I've rebased to include this)
tmlayton
approved these changes
Dec 17, 2018
BPScott
force-pushed
the
cyclical-imports-THEEND
branch
from
8cb2d62 to
719ca66
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
The final cyclical import is dead (again). We can now enable the import/no-cycle eslint rule (again).
#691 removed some cyclical imports. It also stopped exporting a few color helper functions in the final build (see #718) as I incorrectly assumed that they should be internal. It turns out people are using them, so that PR got reverted in #721.
WHAT is this pull request doing?
I reapplied the not-the-color-helper parts of #691 in #754. This PR is the follow up that removes the cyclical dependency in color helpers, without touching any exports.
It moves the color types into the utility folder next to the color utility functions
How to 🎩
yarn run sk type-checkto prove typescript is happyyarn build-consumer webthen runyarn run sk type-checkin web to prove there are no missing exports (You may have to install polaris-icons in web as a workaround because build-consumer isn't smart enough to handle updating transitive dependencies) - web consumes the HSBColor type in some places so this will prove that the types are still exported.