-
Notifications
You must be signed in to change notification settings - Fork 6.8k
chore: make compatibility mode work for apps with multiple modules #2093
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
Conversation
85164e7
to
8c317e3
Compare
@kara rebased and updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Curious that we have that list of selectors, is that to be maintained by us whenever we add/remove component selectors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
/** Directive that enforces that the `mat-` prefix cannot be used. */ | ||
@Directive({selector: MAT_ELEMENTS_SELECTOR}) | ||
export class MatPrefixEnforcer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: From the name, I originally thought that this would enforce that you only had mat- prefixes, rather than you can't have mat- prefixes (as in, you're enforcing that they're present). I think it's the word "Enforcer". "MatPrefixRejector"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, LGTM (although it was LGTM before too)
please rebase |
whoops, tried using the github button to resolve but that was apparently a bad idea, now it complains about merge conflicts when I try to g3sync presubmit. @jelbourn is this something that can be fixed or do i just have to manually sync it now? |
ad80d1d
to
e0ca485
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
R: @kara
The way I previously had the compatibility mode set up, it wouldn't have worked for applications that are composed of multiple NgModules because the sub-modules wouldn't have included the
MdPrefixEnforcer
directive.For this new way, you're always going to have one of the prefix-enforcer directives loaded, but not the other.