feat(cdk): switch injectables to new scope API #10301
Conversation
jelbourn
added
pr: needs review
target: major
jelbourn
requested review from
andrewseguin,
crisbeto,
devversion,
josephperrott and
tinayuangao
as code owners
googlebot
added
the
cla: yes
| }; | ||
|
|
||
| new InjectionToken<() => ScrollStrategy>('cdk-connected-overlay-scroll-strategy', { | ||
| scope: APP_ROOT_SCOPE, |
| @NgModule({ | ||
| providers: [Platform] | ||
| }) | ||
| @NgModule() |
There was a problem hiding this comment.
Out of curiosity, will we be able to kill this module off in the future? It appears to do nothing now
There was a problem hiding this comment.
I suppose so for now, though we could add other stuff to it. Maybe something like
*cdkIfPlatform="IE11, Safari"
| FocusTrapFactory, | ||
| AriaDescriber, | ||
| LIVE_ANNOUNCER_PROVIDER, | ||
| ARIA_DESCRIBER_PROVIDER, |
There was a problem hiding this comment.
I'm not super familiar with how the app-wide providers work yet, but should the AriaDescriber still be set in the providers in this case? I believe that it's the only reason why, for example, the tooltip imports the A11yModule.
There was a problem hiding this comment.
As I understand it, module imports just for this kind of injectable shouldn't be necessary any more. I'll address those in my next PR that applies this to material.
| * | ||
| * @docs-private | ||
| */ | ||
| export const DIR_DOCUMENT = new InjectionToken<Document>('cdk-dir-doc', { |
There was a problem hiding this comment.
As an aside, do we really need a separate token for the document here? We can easily stub out the DOCUMENT itself in the tests.
There was a problem hiding this comment.
When I was originally doing this I found that re-providing common's DOCUMENT with something else made Angular not work.
jelbourn
force-pushed
the
cdk-tree-shakable-services
branch
2 times, most recently
from
52b092f to
60e63ea
Compare
|
I've re-added the old providers because some people were depending on them |
jelbourn
force-pushed
the
cdk-tree-shakable-services
branch
3 times, most recently
from
bea96c7 to
c5b785a
Compare
jelbourn
force-pushed
the
cdk-tree-shakable-services
branch
from
c5b785a to
5a334d7
Compare
jelbourn
force-pushed
the
cdk-tree-shakable-services
branch
from
fdafeb4 to
aae7243
Compare
andrewseguin
added
pr: lgtm
action: merge
|
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. |
No description provided.