Skip to content
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(elements): add ComponentNgElementStrategy to public API #37416

Closed

Conversation

remackgeek
Copy link
Contributor

Adds ComponentNgElementStrategy, ComponentNgElementStrategyFactory to the public API,
allowing third-party libraries to extend the default strategy behavior
(api docs to come in a separate PR)

Fixes #26112, #33490

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • [x ] Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #26112, #33490

What is the new behavior?

ComponentNgElementStrategy, ComponentNgElementStrategyFactory are now public

Does this PR introduce a breaking change?

  • Yes
  • [x ] No

Other information

Adds ComponentNgElementStrategy, ComponentNgElementStrategyFactory to the public API,
allowing third-party libraries to extend the default strategy behavior

Fixes angular#26112, angular#33490
@IgorMinar
Copy link
Contributor

Hi there. Thank you for your contribution to Angular.

I'm sorry but at the moment we are not interested in allowing the default strategy to be part of our public API and to be extended by third parties because it would tie our hands in improving and modifying this default in the future.

Please feel free to implement the NgElementStrategy strategy api and feel free to use the ComponentNgElementStrategy as an inspiration for your implementation.

@IgorMinar IgorMinar closed this Jun 3, 2020
@remackgeek
Copy link
Contributor Author

It was suggested yesterday by @gkalpak, in #24181, based on the publicApi annotations in the code.

If you don't want to expose the strategy, just exposing the ComponentNgElementStrategyFactory (and changing the create() signature to return NgElementStrategy) would let you change the strategy at will, but still let library developers wrap existing behavior.
Right now to do this, I have to create a dummy element with the default strategy, just so I can reach into it and get the strategy to wrap it.

@IgorMinar
Copy link
Contributor

thanks for the additional context @remackgeek. I just spoke with @gkalpak and suggested some alternative solutions. He's going to post a comment with the details. I think the problem you are trying to fix is worth fixing, it's just that the change you proposed here is not heading in the right direction. @gkalpak will share more details. thanks!

@gkalpak
Copy link
Member

gkalpak commented Jun 23, 2020

So, basically, making the strategy (and corresponding factory) part of the public API will increase the maintenance cost by making it harder for us to make changes to them in the future (either fixinf bugs or making improvements).

If anyone wants to define their own strategy, they need to implement the NgElementStrategy interface. This makes things a little more complicated/verbose for people wanting to make a small change to the ComponentNgElementStrategy by entending it, but we feel the benefits are worth it.

Regarding the problem with Zones (#24181) in particular, we decided it is worth fixing in @angular/elements, i.e. with something similar to #24185. So, let's continue the discussion on #24185.

@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 Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Angular Elements]: Adding ComponentNgElementStrategyFactory & ComponentNgElementStrategy to public API
4 participants