Skip to content

bundle of 3 ngtsc fixes for the Material app #25425

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

Closed
wants to merge 3 commits into from

Conversation

alxhub
Copy link
Member

@alxhub alxhub commented Aug 10, 2018

No description provided.

alxhub added 3 commits August 10, 2018 13:23
When an Angular decorated class is inherited, it might be the case that
the entire inheritance chain actually has no constructor defined. In
that event, a factory which simply instantiates the type without any
arguments should be used.
A small bug caused base factory variable statements for @component to
not be emitted properly. At the same time as this is fixed, those
statements are now emitted as const.
@mary-poppins
Copy link

You can preview 4fa9e3a at https://pr25425-4fa9e3a.ngbuilds.io/.

@alxhub alxhub added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Aug 10, 2018
throw new Error(`Type ${proto.name} does not support inheritance`);
if (factory !== null) {
return factory;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for else after return if you care about that d:

Copy link
Member Author

Choose a reason for hiding this comment

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

Very true :) I actually prefer the else here, though.

In general, if the two cases are "equal" in the sense that they're both fundamental branches of the logic, I write the else case out explicitly.

If the if branch is handling some special case by doing an early return, though, and the else case really represents the main flow of the logic, then I'll allow the if to fall through and not explicitly wrap the main logic with else.

@benlesh
Copy link
Contributor

benlesh commented Aug 14, 2018

presubmit

@benlesh benlesh closed this in 82e2725 Aug 14, 2018
benlesh pushed a commit that referenced this pull request Aug 14, 2018
A small bug caused base factory variable statements for @component to
not be emitted properly. At the same time as this is fixed, those
statements are now emitted as const.

PR Close #25425
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
When an Angular decorated class is inherited, it might be the case that
the entire inheritance chain actually has no constructor defined. In
that event, a factory which simply instantiates the type without any
arguments should be used.

PR Close angular#25425
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
)

A small bug caused base factory variable statements for @component to
not be emitted properly. At the same time as this is fixed, those
statements are now emitted as const.

PR Close angular#25425
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
@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 Sep 13, 2019
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 target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants