Skip to content

refactor: assert value for RxJS v7 compatibility #19569

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

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

MichaelJamesParsons
Copy link
Collaborator

In RxJS v7, the type of Subject.next() will change from next(value?: T) to next(value: T) (change). No runtime behavior is affected, but this change will break users who build from source. This PR corrects types to ensure projects build successfully.

Changes

  • asserts T|undefined|null values to T where appropriate.
  • refactor instances of Subject<any> to Subject<void> so the next() method can be called without passing an argument.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 8, 2020
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

@mmalerba PTAL

@jelbourn jelbourn requested a review from mmalerba June 9, 2020 01:37
@jelbourn
Copy link
Member

jelbourn commented Jun 9, 2020

All the void changes look good, Miles should weigh in on the virtual scroll change

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelJamesParsons MichaelJamesParsons force-pushed the rxjs-upgrade branch 2 times, most recently from d45640d to 2dd93f4 Compare June 17, 2020 03:27
@mmalerba mmalerba added lgtm action: merge The PR is ready for merge by the caretaker labels Jun 17, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@mmalerba mmalerba added the target: patch This PR is targeted for the next patch release label Jun 17, 2020
@jelbourn jelbourn added the G This is is related to a Google internal issue label Jun 29, 2020
@jelbourn
Copy link
Member

@MichaelJamesParsons can you update the API gold for snackbar?

yarn bazel run //tools/public_api_guard:material/snack-bar.d.ts_api.accept

@crisbeto
Copy link
Member

The shorthand for updating the golden is yarn approve-api material/snack-bar.

@MichaelJamesParsons
Copy link
Collaborator Author

Done

@jelbourn jelbourn merged commit 1106cb1 into angular:master Jun 30, 2020
@MichaelJamesParsons MichaelJamesParsons deleted the rxjs-upgrade branch July 1, 2020 21:02
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement G This is is related to a Google internal issue target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants