Skip to content

refactor(overlay): avoid circular type references #9907

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 7, 2018
Merged

refactor(overlay): avoid circular type references #9907

merged 1 commit into from
Jun 7, 2018

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Feb 12, 2018

Currently there is a circular type reference between OverlayRef, PositionStrategy/ScrollStrategy and OverlayConfig which can cause issues with some build tools. These changes introduce the OverlayRefBase type to avoid the circular reference.

Fixes #9491.

@crisbeto crisbeto requested a review from jelbourn as a code owner February 12, 2018 20:30
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 12, 2018
@jelbourn
Copy link
Member

jelbourn commented Feb 13, 2018

I tried this out internally and I think it fixes one issue, but I still end up with another issue. I need to consult with Martin to interpret it.

@crisbeto crisbeto force-pushed the 9491/overlay-circular-references branch from aee51c9 to 41b77d7 Compare March 11, 2018 02:10
@jelbourn
Copy link
Member

jelbourn commented Jun 1, 2018

@crisbeto could you rebase this one? I want to do another presubmit to remember what the problem was

@crisbeto crisbeto force-pushed the 9491/overlay-circular-references branch from 41b77d7 to 99e8b2d Compare June 2, 2018 16:27
@crisbeto
Copy link
Member Author

crisbeto commented Jun 2, 2018

Done. It needed a bit more work to get it up-to-speed with master.

jelbourn
jelbourn approved these changes Jun 5, 2018
View reviewed changes
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.

LGTM

Just ran a presubmit and this is green.

I don't love the name, but I don't have a better suggestion (maybe writing out OverlayReference?)

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jun 5, 2018
@crisbeto
refactor(overlay): avoid circular type references
fc78a3d 
Currently there is a circular type reference between `OverlayRef`, `PositionStrategy`/`ScrollStrategy` and `OverlayConfig` which can cause issues with some build tools. These changes introduce the `OverlayRefBase` type to avoid the circular reference.

Fixes angular#9491.
@crisbeto crisbeto force-pushed the 9491/overlay-circular-references branch from 99e8b2d to fc78a3d Compare June 6, 2018 19:14
@crisbeto
Copy link
Member Author

crisbeto commented Jun 6, 2018

Renamed to OverlayReference.

@andrewseguin andrewseguin merged commit ed0de40 into angular:master Jun 7, 2018
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Sep 9, 2019

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 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jelbourn jelbourn jelbourn approved these changes

Assignees

@jelbourn jelbourn

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Circular type references in material2

4 participants

@crisbeto @jelbourn @googlebot @andrewseguin