Skip to content
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

chore: update deprecations #6259

Merged
merged 38 commits into from Apr 27, 2021
Merged

Conversation

cartant
Copy link
Collaborator

@cartant cartant commented Apr 25, 2021

Description:

This PR updates the @deprecation tags in the TSDoc. Most of the deprecation messages were sufficiently informative, but some were confusing. And, coupled with the false positives that used to be effected, this seemed to confused some folks - well, it resulted in numerous issues being opened.

The changes in the PR make the messages more consistent and informative. Where appropriate links to the deprecation docs are provided so that devs can read a more detailed explanation of what's deprecated and what refactoring is necessary.

I've edited the messages in an attempt to optimize them for display in VS Code, as that's where they're going to be read most often, IMO. VS Code collapses whitespace within the message and displays the message as single, wrapped line - with code formatting and proper links to the deprecation details. It leaves the TSDoc {@link whatever} tags as they are - which is unfortunate, but we'll just have to live with it, I suppose. I've removed a fair bit of verbosity - 'please', lots of adjectives, etc. - so that the messages are shorter and are less overwhelming when shown as a single line.

I've created a multicasting.md deprecations document that has refactor examples for all of the multicasting deprecations. They are much easier to read in that format. And there is more context.

IMPORTANT: the multicasting deprecations and refactors use an upcoming change to the connectable API - it's going to get a config object with connector and resetOnDisconnect properties. This change will be in an upcoming PR (I've discussed this with Ben) - see #6267.

Some specific changes:

  • This PR remove the unused and unexported $$iterator symbol
  • The defer TSDoc has been updated to use the same type - ObservableInput - that's used in the actual API
  • delayWhen had a signature that only existed so that a deprecation could be applied. That deprecation was specific to v6. AFAICT, the deprecation was supposed to serve as a warning, but the behaviour about which it was warning has now been changed. I've removed that signature.
  • I've removed the materialize deprecation. IMO, we cannot use a deprecation on an operator to warn people about the values it emits. The operator is not deprecated. There are deprecations on the methods of Notification, so if people call those methods, they'll see deprecations at the call site. Deprecating an operator that is not going to be removed will be confusing and counterproductive, IMO.
  • I've removed the deprecation from sampleTime 'cause auditTime, et al. are not deprecated. If we really want to deprecate this, we can do it later - after a discussion.
  • I've used HTML comments to disable Prettier from formatting snippets in multicasting.md 'cause it's too hard to read long lines of 'pretty' code.

There are additional deprecations that I think need to be added - as there are several places in the API in which either an array of sources or rest parameters can be passed - but I will include these in a separate PR.

Related issue (if exists): Nope

@cartant cartant changed the title chore: update deprecations WIP: chore: update deprecations Apr 25, 2021
@cartant cartant marked this pull request as ready for review April 27, 2021 09:17
@cartant cartant changed the title WIP: chore: update deprecations chore: update deprecations Apr 27, 2021
@cartant cartant requested a review from benlesh April 27, 2021 09:17
Copy link
Member

@niklas-wortmann niklas-wortmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benlesh benlesh merged commit 1f7b9c5 into ReactiveX:master Apr 27, 2021
@cartant cartant deleted the cartant/deprecations branch May 2, 2021 04:59
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.

None yet

3 participants