chore: add runtime component deprecation warnings by jcfranco · Pull Request #9821 · Esri/calcite-design-system

jcfranco · 2024-07-20T00:49:43Z

Related Issue: #9295

Summary

This adds deprecation warning messages to components marked for deprecation at v3 and v4.

Notes

Adds logger util to:
- facilitate deprecation of component and their members (this PR)
- help ensure consistent log messages across components and utils (follow-up PR)
- display a 'badge' for calcite-related messages (similar to Stencil)
Fixes typo in variable for global test log spy
Adds logLevel to the config for users to specify a different log level
There is a known limitation for composite components where the host and supporting components are also deprecated. For these cases, a deprecation message will be displayed per component.

Questions

Badge 👍👎?
Assuming 👍 on the badge, should we keep calcite in component names used in messaging? The badge already provides context
Do we want to keep logLevel in the config or should we do something similar to Maps SDK's config for future log-related features? I don't see a use case for interceptors at the moment.

driskull · 2024-07-22T15:39:47Z

I do like the badge
I don't think we need the calcite prefix as it is redundant with the badge.
I think its probably fine in the config for now. What do you think?

driskull

This will be awesome! 💯

driskull · 2024-07-22T15:36:03Z

+import { logger } from "../../utils/logger";
 import { CSS, SLOTS } from "./resources";

+logger.deprecated("component", {


this is super cool!

driskull · 2024-07-22T15:38:15Z

+  }
+
+  const badgeTemplate = "%ccalcite";
+  const badgeStyle = "background: #007AC2; color: #fff; border-radius: 4px;padding: 2px 4px;";


Should design approve the styling :)

Wondering if we could use CSS vars here instead of styles. I guess it doesn't really matter since its just a log badge but still.

I did test this, but no dice. Custom CSS props don't appear to be available for console styling. 😞

driskull · 2024-07-22T15:38:53Z

+    listFormatter = new Intl.ListFormat("en", { style: "long", type: "disjunction" });
+  }
+
+  const message = `[${name}] ${context} is deprecated and will be removed in ${removalVersion === "future" ? `a future version` : `v${removalVersion}`}.${suggested ? ` Use ${multiSuggestions ? listFormatter.format(suggested.map((suggestion) => `"${suggestion}"`)) : `"${suggested}"`} instead.` : ""}`;


We don't need to translate this right?

Correct. Dev messaging is not translated.

DitwanP · 2024-07-22T19:20:59Z

This is great! 💪 👏

I'm with Matt on points 1, 2, and 3 from his first comment above.

geospatialem

Awesome!! 💪🏻

As far as the version, should we also mention a timeframe for folks to plan, or keep it vague with the version? Something like:

calcite [tip-group] component is deprecated and will be removed in v4 (2025 Q4). Use "carousel" or "carousel item" instead.

Badge 👍👎?

Badge it up! 🦡📛

Assuming 👍 on the badge, should we keep calcite in component names used in messaging? The badge already provides context

For consistency with our documentation, its okay to omit since its mentioned already in the badge. If we remove the badge, we should leave it in though.

The only hesitation would be if omitting it creates confusion for developers to use the recommended component(s).

It looks like the messaging also adds " to the recommended components, should the symbol be around each of the recommendations, like:

Use "carousel" or "carousel-item" instead.

Do we want to keep logLevel in the config or should we do something similar to Maps SDK's config for future log-related features? I don't see a use case for interceptors at the moment.

Flexible on whichever approach is best as long as it is clear to developers working with both Maps SDK and Calcite.

benelan

I like this idea as well! One suggestion that hasn't been mentioned, maybe add a "none" or "disabled" option to logLevels? That way teams can completely turn it off in prod and set it to something verbose in dev.

jcfranco · 2024-07-23T00:49:16Z

As far as the version, should we also mention a timeframe for folks to plan, or keep it vague with the version? Something like:

I like the idea of adding the timeframe, but this would be tricky to maintain and enforce via TypeScript.

The only hesitation would be if omitting it creates confusion for developers to use the recommended component(s).

I hear ya. We should be good on this front since we do have the badge giving context and often omit calcite in the doc when referring to components. This also makes the message less noisy.

It looks like the messaging also adds " to the recommended components, should the symbol be around each of the recommendations, like:

My bad. 😅 I used an outdated screenshot. Each component should be quoted in the latest version (covered by a test too).

jcfranco · 2024-07-23T00:49:21Z

Thanks for the feedback, y'all! 🚀 Made the following changes:

dropped namespace from component messaging to simplify (this is enforced by TS)
added an off level to mute all logs

eriklharper · 2024-07-23T16:49:54Z

+1 for badge, +1 for keeping calcite in the name of the component message. I think that's clearer for folks outside the core team since that's how they interact with the components. Keeping log level in the config sounds like a good first step, we can always add more to that down the road.

…-to-monorepo * origin/dev: chore: release next fix(tabs): handle tab close events that remove the associated tab-title and tab elements from the DOM (#9768) chore: add runtime component deprecation warnings (#9821) build(deps): drop unnecessary @types/prettier dep (#9833) build: update browserslist db (#9822) build(deps): update dependency dayjs to v1.11.12 (#9824) build(deps): update dependency semver to v7.6.3 (#9825)

chore: add messaging to deprecated components

34d34c4

github-actions bot assigned jcfranco Jul 20, 2024

jcfranco added the skip visual snapshots Pull requests that do not need visual regression testing. label Jul 20, 2024

github-actions bot added the chore Issues with changes that don't modify src or test files. label Jul 20, 2024

jcfranco requested review from benelan and driskull July 20, 2024 01:04

driskull approved these changes Jul 22, 2024

View reviewed changes

geospatialem self-requested a review July 22, 2024 19:30

geospatialem reviewed Jul 22, 2024

View reviewed changes

benelan approved these changes Jul 22, 2024

View reviewed changes

jcfranco added 5 commits July 22, 2024 16:35

add default config log level test

5c01c41

add none and trace levels along with tests

bc45b28

tidy up

43f26ea

drop namespace from deprecation messages

164241b

enforce no-namespace via type

a92ed98

jcfranco added 6 commits July 22, 2024 17:53

fix incorrect component name argument

87ecd7f

add missing TestGlobal type

108f13a

update spec test params

343c32f

fix spec types

50656e5

expect TS errors for fake names

f198129

move misplaced ts-expect-error line

6122f11

jcfranco merged commit 574ac9a into dev Jul 23, 2024

jcfranco deleted the jcfranco/9295-add-deprecation-messaging branch July 23, 2024 03:33

github-actions bot added this to the 2024-07-30 - Jul Release milestone Jul 23, 2024

calcite-admin pushed a commit that referenced this pull request Jul 30, 2024

chore: add runtime component deprecation warnings (#9821)

85fbd9c

Conversation

jcfranco commented Jul 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Questions

Uh oh!

driskull commented Jul 22, 2024

Uh oh!

driskull left a comment

Choose a reason for hiding this comment

Uh oh!

driskull Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

driskull Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

jcfranco Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

driskull Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

jcfranco Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

DitwanP commented Jul 22, 2024

Uh oh!

geospatialem left a comment

Choose a reason for hiding this comment

Uh oh!

benelan left a comment

Choose a reason for hiding this comment

Uh oh!

jcfranco commented Jul 23, 2024

Uh oh!

jcfranco commented Jul 23, 2024

Uh oh!

eriklharper commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jcfranco commented Jul 20, 2024 •

edited

Loading