-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] deprecate Toaster in favor of OverlayToaster #6146
Conversation
[core] deprecate Toaster in favor of OverlayToasterBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
// eslint-disable-next-line @typescript-eslint/no-redeclare | ||
export type Toaster = ToasterInstance; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this is a breaking change, e.g. https://github.com/pybricks/pybricks-code/actions/runs/4989185014/jobs/8932767904?pr=1741#step:5:6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, sorry about that, thanks for flagging. I suppose I could fix this by changing it to:
// eslint-disable-next-line @typescript-eslint/no-redeclare | |
export type Toaster = ToasterInstance; | |
// eslint-disable-next-line @typescript-eslint/no-redeclare | |
export type Toaster = typeof OverlayToaster; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened a PR: #6165
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should probably just be deleted until v5 to avoid complications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think removing this line will be backwards-compatible.
Before, Toaster
was a class, so it could be referenced as a type or a value.
After this change, it's re-exported as a value with const Toaster = OverlayToaster
. But that will break existing usage of Toaster
as a type (you could work around this with typeof Toaster
).
To restore Toaster
as a type in the public API, I think we need an export type Toaster = ...
statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 makes sense.
Changes proposed in this pull request:
Properly deprecate
Toaster
API in favor ofOverlayToaster
, for forwards-compatibility with v5.0. See https://github.com/palantir/blueprint/wiki/Toaster-5.0-changesThis is most commonly referenced in user code like this:
Also mark
IToasterProps
as deprecated, to be replaced byOverlayToasterProps
. We can auto-fix this type with theno-deprecated-type-references
lint rule, but we don't have the no-deprecated-components rule set up to auto-fix components right now.