Skip to content

Conversation

@BPScott
Copy link
Member

@BPScott BPScott commented May 8, 2020

WHY are these changes introduced?

Consuming applications currently have to reference two files to use our public sass API. Instead we should get people to use a single entry point now recent (since Nov 2019) versions of sewing-kit no-longer hardcode opinions about what our public api endpoints are named.

WHAT is this pull request doing?

Deprecate using foundation.scss and shared.scss to access our public
sass API. Instead consuming apps should import _public-api.scss.

By exposing a single entrypoint we make it slightly easier for consuming
apps to use our public API.

foundation.scss and shared.scss remain present for now to retain
compatability with sewing-kit versions older than 0.113.0 (released
Nov 2019), as older versions hardcoded those paths. They should be
deleted in Polaris v6, and v6 should require using sewing-kit >= 0.113.0

How to 🎩

Do a build, see that styles/_public-api.scss and esnext/styles/_public-api.scss are created, and note that they contain the same imports as foundation.scss and shared.scss combined.

Who does this affect and how?

Importing _public-api.scss is functionally identical to importing foundation.scss and shared.scss.

Consuming applications using recent versions of sewing-kit (>=0.113.0)

Modern SK versions require consuming apps to explicitly configure usage of our public sass api if they wish to use it. Here is how web does it.

Instead of importing esnext/styles/foundation.scss and esnext/styles/shared.scss, they should import esnext/styles/_public-api.scss instead.

- join(polarisStyles, 'foundation.scss'),
- join(polarisStyles, 'shared.scss'),
+ join(polarisStyles, '_public-api.scss'),

Consuming applications using old versions of sewing-kit (< 0.113.0)

These old versions of sewing-kit implicitly configure the import of esnext/styles/foundation.scss and esnext/styles/shared.scss instead of making the App do it. As those two files continue to exist things will be fine until we remove those files in v6.

At some point before that they will need to update to a version of sewing-kit >=0.113.0 and then configure their autoImport settings per the sewing-kit migration guide where they should point to esnext/styles/_public-api.scss.

Consumers not using sewing-kit at all

Any other consumer is getting at our API by importing styles/foundation.scss and styles/shared.scss, they should import styles/_public-api.scss instead. (that's not a typo remember that scss will look for partial files prefixed with an underscore)

- @import '~@shopify/polaris/styles/foundation';
- @import '~@shopify/polaris/styles/shared';
+ @import '~@shopify/polaris/styles/public-api';

@BPScott BPScott requested a review from danrosenthal May 8, 2020 02:13
@BPScott BPScott requested a review from tmlayton as a code owner May 8, 2020 02:13
@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2020

🟢 No significant changes to src/**/*.tsx were detected.

@tmlayton
Copy link
Contributor

tmlayton commented May 8, 2020

Just a heads up I won’t have time to look at this until next week.

Deprecate using foundation.scss and shared.scss to access our public
sass API. Instead consuming apps should import `_public-api.scss`.

By exposing a single entrypoint we make it slightly easier for consuming
apps to use our public API.

foundation.scss and shared.scss remain present for now to retain
compatability with sewing-kit versions older than 0.113.0 (released
Nov 2019), as older versions hardcoded those paths. They should be
deleted in Polaris v6, and v6 should require using sewing-kit >= 0.113.0
@BPScott BPScott force-pushed the sass-single-public-entrypoint branch from 58faa20 to 39b6be4 Compare May 8, 2020 06:13
@BPScott BPScott merged commit b5b71cb into master May 11, 2020
@BPScott BPScott deleted the sass-single-public-entrypoint branch May 11, 2020 19:49
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