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.

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

Conversation

crisbeto
Copy link
Member

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

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.

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
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

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.
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
Development

Successfully merging this pull request may close these issues.

Circular type references in material2
4 participants