Skip to content

Support for input transforms in @angular/elements#50713

Closed
crisbeto wants to merge 2 commits intoangular:mainfrom
crisbeto:50708/elements-input-transforms
Closed

Support for input transforms in @angular/elements#50713
crisbeto wants to merge 2 commits intoangular:mainfrom
crisbeto:50708/elements-input-transforms

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Jun 14, 2023

Split up across a couple of commits that aim to fix that input transforms weren't being used in @angular/elements. Includes:

  1. Exposing the input transform function in ComponentFactory.inputs and ComponentMirror.inputs.
  2. Updating elements to account for the transform function.

Fixes #50708.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release area: elements Issues related to Angular Elements labels Jun 14, 2023
@ngbot ngbot bot modified the milestone: Backlog Jun 14, 2023
@crisbeto crisbeto force-pushed the 50708/elements-input-transforms branch from aa2b924 to 692e07b Compare June 14, 2023 11:05
@crisbeto crisbeto marked this pull request as ready for review June 14, 2023 11:20
@pullapprove pullapprove bot requested a review from jessicajaniuk June 14, 2023 11:20
ajafff
ajafff reviewed Jun 14, 2023
View reviewed changes
packages/core/src/render3/component_ref.ts Outdated
Copy link
Copy Markdown

@ajafff ajafff Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer Object.hasOwn. otherwise this will break if input.propName === 'hasOwnProperty' and that input has a transform

jessicajaniuk
jessicajaniuk approved these changes Jun 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

reviewed-for: fw-core, fw-elements, public-api

@jessicajaniuk jessicajaniuk removed their request for review June 20, 2023 16:20
@pullapprove pullapprove bot requested a review from dylhunn June 20, 2023 16:20
dylhunn
dylhunn approved these changes Jun 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: fw-core, public-api

AndrewKushnir
AndrewKushnir approved these changes Jun 20, 2023
View reviewed changes
@pkozlowski-opensource pkozlowski-opensource removed their request for review June 22, 2023 06:55
@pkozlowski-opensource pkozlowski-opensource added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 22, 2023
@ngbot
Copy link
Copy Markdown

ngbot bot commented Jun 22, 2023

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google-internal-tests" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

crisbeto added 2 commits June 22, 2023 09:52
@crisbeto
fix(core): expose input transform function on ComponentFactory and Co…
f07d63a 
…mponentMirror

Exposes the function used to transform an input on `ComponentFactory.inputs` and `ComponentMirror.inputs`. We'll need this to support input transforms in `elements`.
@crisbeto
fix(elements): support input transform functions
6e05186 
Fixes that `@angular/elements` didn't support input transform functions.

Fixes angular#50708.
@crisbeto crisbeto force-pushed the 50708/elements-input-transforms branch from 692e07b to 6e05186 Compare June 22, 2023 07:52
@crisbeto crisbeto added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jun 22, 2023
@crisbeto
Copy link
Copy Markdown
Member Author

crisbeto commented Jun 22, 2023

Caretaker note: the TAP failures are some pre-existing failures in unrelated targets.

@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Jun 22, 2023

This PR was merged into the repository by commit d64864e.

AndrewKushnir pushed a commit that referenced this pull request Jun 22, 2023
@crisbeto @AndrewKushnir
fix(core): expose input transform function on ComponentFactory and Co…
dd6fc57 
…mponentMirror (#50713)

Exposes the function used to transform an input on `ComponentFactory.inputs` and `ComponentMirror.inputs`. We'll need this to support input transforms in `elements`.

PR Close #50713
AndrewKushnir pushed a commit that referenced this pull request Jun 22, 2023
@crisbeto @AndrewKushnir
fix(elements): support input transform functions (#50713)
e1bbe47 
Fixes that `@angular/elements` didn't support input transform functions.

Fixes #50708.

PR Close #50713
AndrewKushnir pushed a commit that referenced this pull request Jun 22, 2023
@crisbeto @AndrewKushnir
fix(elements): support input transform functions (#50713)
d64864e 
Fixes that `@angular/elements` didn't support input transform functions.

Fixes #50708.

PR Close #50713
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Jul 23, 2023

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 Jul 23, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@crisbeto @ChellappanRajan
fix(core): expose input transform function on ComponentFactory and Co…
3b29de1 
…mponentMirror (angular#50713)

Exposes the function used to transform an input on `ComponentFactory.inputs` and `ComponentMirror.inputs`. We'll need this to support input transforms in `elements`.

PR Close angular#50713
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@crisbeto @ChellappanRajan
fix(elements): support input transform functions (angular#50713)
25aa296 
Fixes that `@angular/elements` didn't support input transform functions.

Fixes angular#50708.

PR Close angular#50713
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

+3 more reviewers

@ajafff ajafff ajafff left review comments

@dylhunn dylhunn dylhunn approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime area: elements Issues related to Angular Elements merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Input transform not working with Angular Elements

6 participants

@crisbeto @AndrewKushnir @dylhunn @ajafff @jessicajaniuk @pkozlowski-opensource