Add documentation for deprecating styles. #38540
Conversation
mirka
added
[Type] Developer Documentation
| @@ -180,6 +183,65 @@ All new component should be styled using [Emotion](https://emotion.sh/docs/intro | |||
|
|
|||
| Note: Instead of using Emotion's standard `cx` function, the custom [`useCx` hook](/packages/components/src/utils/hooks/use-cx.ts) should be used instead. | |||
|
|
|||
|
|
|||
| ### Deprecating styles | |||
There was a problem hiding this comment.
Should this deprecation strategy be recommended also beyond styles? It seems like a reasonable strategy to use also for behavioural changes, API changes, etc...
There was a problem hiding this comment.
I do see this as a special case of deprecating APIs, where there is no actual prop to deprecate and it's just a change in default behavior. Looking at the instances in components/* where deprecated() is called, I'm not sure this kind of "change in default" deprecation has ever happened. Are you aware of any?
I did choose the __next* prefix so it would be appropriate for behavioral changes as well though, so it probably would be generally applicable when it happens. I think we could reuse or refactor this section when we write a more detailed deprecation guide for components.
By the way I just learned about this section that asks us to add a "Needs Dev Note" label to the PR. I'll add this into the doc.
There was a problem hiding this comment.
Are you aware of any?
On top of my mind, no. Hey @sarayourfriend (sorry for the ping!) — do you ever remember an instance of a deprecation where the meaning of a style/prop was changed (causing a potentially breaking change)?
I did choose the __next* prefix so it would be appropriate for behavioral changes as well though, so it probably would be generally applicable when it happens. I think we could reuse or refactor this section when we write a more detailed deprecation guide for components.
Sounds good to me. Having a defined way to "safely" introduce a breaking change is great and we should absolutely iterate on this procedure in order to be able to apply it to more deprecation scenarios
By the way I just learned about this section that asks us to add a "Needs Dev Note" label to the PR. I'll add this into the doc.
TIL !
There was a problem hiding this comment.
do you ever remember an instance of a deprecation where the meaning of a style/prop was changed (causing a potentially breaking change)
I can't think of any from my time working on wp/components but that's mostly because I'm fairly certain we only "soft" deprecated things.
We did change the variant prop on the Text component, but it was experimental. We originally wrote an adapter for it IIRC but it isn't there anymore, or maybe it was never merged because the component was experimental anyway.
Although, does this count? #33490
There was a problem hiding this comment.
Yeah, that's what I thought as well.
Although, does this count? #33490
Not sure if it does, I'd probably see it as more of a bug fix — isScrollable was implicitly introduced to CardBody in a previous PR, and the PR that you highlighted changes its default value to fix that regression.
There was a problem hiding this comment.
Right right... I feel like I remember seeing a bug raised where we'd accidentally left a deprecation or prop-meaning change out of the changelogs somewhere and it'd caused problems for someone consuming the package. Just can't remember how to find it now 😞
There was a problem hiding this comment.
I believe the original "breaking change" was caused by my refactor of Card in #32566
There was a problem hiding this comment.
Ahhh yes! That is what I was remembering. But it was just an accidental prop value rename 😬 Doesn't really match what we're talking about here.
mirka
force-pushed
the
docs/deprecating-styles
branch
from
19a3cad to
e45104a
Compare
cbravobernal
changed the title
cbravobernal
changed the title
Description
Add a "Deprecating styles" section to the wp-components contributors doc.
I mostly want to discuss and align on the style deprecation strategy here, not necessarily saying we have to include it in the devdocs. I thought through a bunch of options, and this is the strategy I am proposing. Thoughts?
Types of changes
Docs only.