Use unpkg for CDN links for CSS files #1960
Conversation
|
This is a draft PR for now. Once #1957 is merged and a new RC is released, I shall rebase this atop latest master and confirm it is ready for review. |
BPScott
force-pushed
the
use-unpkg-for-cdn
branch
from
8466d29 to
3d8e4c0
Compare
BPScott
force-pushed
the
use-unpkg-for-cdn
branch
from
3d8e4c0 to
b4f648d
Compare
BPScott
force-pushed
the
use-unpkg-for-cdn
branch
from
b4f648d to
0c7d578
Compare
- Stop publishing css files to sdks.shopifycdn.com - Update docs to use unpkg link for CCSS files - Make it clear that we sugggest downloading these files and hosting them yourself rather than using unpkg directly
BPScott
force-pushed
the
use-unpkg-for-cdn
branch
from
0c7d578 to
1b54456
Compare
| import '@shopify/polaris/styles.css'; | ||
| ``` | ||
|
|
||
| Otherwise include the CSS in your HTML. We suggest copying the styles file into your own project, but you may also use it directly: |
There was a problem hiding this comment.
We suggest copying the styles file into your own project
Do we want to recommend this because we don't want to be tied to unpkg? I think if we clarify why we suggest this, it will help people understand why it would be worth copying vs linking in their HTML.
There was a problem hiding this comment.
I'm thinking more along the lines that generally don't want to add an extra point of failure your app - why pull in stuff from another host when you can your own.
That tradeoff affects all 3rd party code it doesn't seem that polaris specific so i left it out as it felt like already implied knowledge.
How about: We suggest copying the styles file into your own project to avoid adding extra hosts you depend upon, but you may also use it directly?
There was a problem hiding this comment.
I guess I just find it confusing because using a CDN is supposed to be better performance wise than having a bunch of styles local to your app. Having all of the Bootstrap styles local to your app for instance would be insane vs having the CDN link. I'm struggling to find it convincing to consumers as something that would be a useful tradeoff.
extra point of failure your app
If we frame copying it to your app locally as a fallback in case the CDN is down does that makes sense? Or are you thinking of other points of failure?
There was a problem hiding this comment.
This article is handy https://csswizardry.com/2019/05/self-host-your-static-assets/
a CDN is supposed to be better performance wise than having a bunch of styles local to your app.
Broadly speaking yes. However much of those benefits are lost if you you've got one asset on a different host to everything else - it's another host you have to negotiate a connection with and you lose any resource prioritization. The time you lose to those things offset any gain you'd get from using a CDN. It's preferable to keep all your critical static assets on a single host - even if that host isn't fronted by a CDN.
Having all of the Bootstrap styles local to your app for instance would be insane vs having the CDN link.
Indeed it would, however having the single boostrap.min.css file hosted on the same server as your other static assets would be preferable to both, which is the style of hosting we're talking about :)
If we frame copying it to your app locally as a fallback in case the CDN is down does that makes sense?
I think that pattern adds extra complication for negligible additional resilience. I don't think it is worth explaining or promoting it.
There was a problem hiding this comment.
The time you lose to those things offset any gain you'd get from using a CDN. It's preferable to keep all your critical static assets on a single host - even if that host isn't fronted by a CDN.
Ahhh okay, that makes sense. Thank you for clarifying!
| - Added support for React hooks in Storybook ([#1665](https://github.com/Shopify/polaris-react/pull/1665)) | ||
| - Created `toBeDisabled`, `mountWithContext` and added custom testing matchers ([#1596](https://github.com/Shopify/polaris-react/pull/1596)) | ||
| - Enabled strict mode in TypeScript ([#1883](https://github.com/Shopify/polaris-react/pull/1883)) | ||
| - Moved to `unpkg.com` for our CDN CSS assets, instead of using `sdks.shopifycdn.com`. Existing URLs will continue to work but new versions will only be available on `unpkg.com` ([#1960](https://github.com/Shopify/polaris-react/pull/1960)) |
There was a problem hiding this comment.
Is there a section in the change log for breaking changes? Will this change also be included in the upgrade instructions for v4?
There was a problem hiding this comment.
There is a breaking changes section. Do you think it belongs there more? I was torn as existing links will still continue to work, it's only if you want the new stuff the URL has changed.
There was a problem hiding this comment.
I'll also be mentioning this in the migration guide doc that is being written in #1945
There was a problem hiding this comment.
Agreed--since the old links continue to work and it's just moving forward from v4 on that the domain changes, it wouldn't make sense to call this a breaking change 👍
I do think it's worth mentioning in the v4 upgrade instructions though, since people are used to just swapping out the semver version number when they upgrade but this is a whole new host.
|
Gonna merge as is, I've got some follow-up work to tidy up the getting started examples so we can tweak the wording around CSS hosting recommendations in there. |
- Stop publishing css files to sdks.shopifycdn.com - Update docs to use unpkg link for CCSS files - Make it clear that we sugggest downloading these files and hosting them yourself rather than using unpkg directly
2 participants
WHY are these changes introduced?
Maintaining our own CDN distribution of our CSS files is a little annoying as it leads to more complex builds. Let's let unpkg handle our CDN styles (just like IBM's Carbon design system and React).
The first step to that is adding minified styles into npm package - that was #1957
The second step is ripping out our build config that uploads files to sdks.shopifycdn.com and updates our links to unpkg.com files (that's this PR)
WHAT is this pull request doing?
How to 🎩
sdks.shopifycdnis never referenced elsewhere in the project