-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
feat(ivy): add injectable-pipe schematic #29847
feat(ivy): add injectable-pipe schematic #29847
Conversation
75b3b45
to
b4c6a47
Compare
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. Just a few minor nits.
packages/core/schematics/migrations/injectable-pipe/BUILD.bazel
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/injectable-pipe/google3/injectablePipeRule.ts
Outdated
Show resolved
Hide resolved
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, once comments from @devversion are resolved
Just wondering, but can't the |
@alfaproject We considered that, but combining the decorators that way would force all future pipes to generate extra code for injection defs even if they aren't designed to be injected. If we keep them separate, pipes where injection doesn't make sense can save the bytes. Also worth noting that this schematic is a first pass. If we have time, we want to limit the pipes we're decorating to just those that are provided or injected. |
b4c6a47
to
ab58233
Compare
The feedback has been addressed @devversion @kara. |
packages/core/schematics/migrations/injectable-pipe/google3/BUILD.bazel
Outdated
Show resolved
Hide resolved
bdcec57
to
795d6b1
Compare
@crisbeto Can you rebase this? |
Adds a schematic that will annotate all `Pipe` classes with `Injectable` so that they can be injected. This PR resolves FW-1228.
795d6b1
to
c9b96a7
Compare
Rebased. |
c9b96a7
to
53c0a40
Compare
53c0a40
to
c2ba55e
Compare
Adds a schematic that will annotate all `Pipe` classes with `Injectable` so that they can be injected. This PR resolves FW-1228. PR Close angular#29847
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. |
Adds a schematic that will annotate all
Pipe
classes withInjectable
so that they can be injected.This PR resolves FW-1228.