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): implement `@angular/elements` #19469

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
@gkalpak
Member

gkalpak commented Sep 28, 2017

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] 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: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Sep 30, 2017

@mhevery

Few comments. @robwormald will have more feedback for you.

Also I think we decided to cal it just @angular/elements so you will need to do search and replace.

Show outdated Hide outdated packages/custom-elements/src/ng-element-constructor.ts Outdated
Show outdated Hide outdated packages/custom-elements/src/ng-element.ts Outdated
Show outdated Hide outdated packages/custom-elements/src/ng-element.ts Outdated
Show outdated Hide outdated packages/custom-elements/test/ng-element_spec.ts Outdated
@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Oct 3, 2017

@gkalpak gkalpak changed the title from WIP - feat(custom-elements): implement `custom-elements` to WIP - feat(elements): implement `@angular/elements` Oct 4, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Oct 16, 2017

Show outdated Hide outdated karma-js.conf.js Outdated
@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Oct 17, 2017

* TODO(gkalpak): Add docs.
* @experimental
*/
export function registerAsCustomElements<T>(

This comment has been minimized.

@mhevery

mhevery Oct 17, 2017

Member

Change signature per discussion:

registerAsCustomElements(components: Type<any>[], moduleFactory: NgModuleFactory, platform: PlatformRef);
registerAsCustomElements(components: Type<any>[], moduleFactory: () => Promise<NgModuleRef<T>>);
@mhevery

mhevery Oct 17, 2017

Member

Change signature per discussion:

registerAsCustomElements(components: Type<any>[], moduleFactory: NgModuleFactory, platform: PlatformRef);
registerAsCustomElements(components: Type<any>[], moduleFactory: () => Promise<NgModuleRef<T>>);

This comment has been minimized.

@gkalpak

gkalpak Oct 20, 2017

Member

I just realized I have the arguments in reversed order (platformRef before moduleFactory). Should I put the moduleFactory first?

@gkalpak

gkalpak Oct 20, 2017

Member

I just realized I have the arguments in reversed order (platformRef before moduleFactory). Should I put the moduleFactory first?

This comment has been minimized.

@mhevery

mhevery Oct 23, 2017

Member

I think your way of doing is fine. So no change needed.

@mhevery

mhevery Oct 23, 2017

Member

I think your way of doing is fine. So no change needed.

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Oct 20, 2017

accidental approval

Show outdated Hide outdated packages/elements/src/utils.ts Outdated

@angular angular deleted a comment Oct 23, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@angular angular deleted a comment from mary-poppins Oct 26, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Nov 2, 2017

@mary-poppins

This comment has been minimized.

Show comment
Hide comment
@mary-poppins

mary-poppins commented Nov 2, 2017

@vicb

This comment has been minimized.

Show comment
Hide comment
@vicb

vicb Nov 2, 2017

Contributor

@gkalpak is it expected to have a feat on a patch branch (5.0.x) ?

Contributor

vicb commented Nov 2, 2017

@gkalpak is it expected to have a feat on a patch branch (5.0.x) ?

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Nov 2, 2017

Member

Hm...probably not. I had it marked as master-only before the release, but since it didn't get merged, I changed to master & patch so that it lands on the branches that it would have landed if it were merged earlier. Didn't think about the fix-vs-feat distinction.
(I've had too much AngularJS, I guess, where patch releases are treated as minor releases (since there can be no major version bumps) 😁)

Thx for catching this 👍

Member

gkalpak commented Nov 2, 2017

Hm...probably not. I had it marked as master-only before the release, but since it didn't get merged, I changed to master & patch so that it lands on the branches that it would have landed if it were merged earlier. Didn't think about the fix-vs-feat distinction.
(I've had too much AngularJS, I guess, where patch releases are treated as minor releases (since there can be no major version bumps) 😁)

Thx for catching this 👍

@vicb

This comment has been minimized.

Show comment
Hide comment
@vicb

vicb Nov 2, 2017

Contributor

Thanks for you PR!
Target labels are confusing.

Contributor

vicb commented Nov 2, 2017

Thanks for you PR!
Target labels are confusing.

@vicb vicb closed this in dcf8840 Nov 2, 2017

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Nov 3, 2017

Member

🎉

Member

gkalpak commented Nov 3, 2017

🎉

@gkalpak gkalpak deleted the gkalpak:feat-custom-elements branch Nov 3, 2017

IgorMinar added a commit to IgorMinar/angular that referenced this pull request Nov 3, 2017

revert: @angular/elements PR angular#19469
This PR was merged without API docs and general rollout plan.

We can't release this as is in 5.1 without a plan for documentation, cli integration, etc.

IgorMinar added a commit to IgorMinar/angular that referenced this pull request Nov 3, 2017

revert: feat(elements): implement `@angular/elements` angular#19469
This PR was merged without API docs and general rollout plan.

We can't release this as is in 5.1 without a plan for documentation, cli integration, etc.

IgorMinar added a commit that referenced this pull request Nov 3, 2017

revert: feat(elements): implement `@angular/elements` #19469
This PR was merged without API docs and general rollout plan.

We can't release this as is in 5.1 without a plan for documentation, cli integration, etc.

vicb added a commit that referenced this pull request Nov 3, 2017

revert: feat(elements): implement `@angular/elements` #19469 (#20152)
This PR was merged without API docs and general rollout plan.

We can't release this as is in 5.1 without a plan for documentation, cli integration, etc.
@MarkPieszak

This comment has been minimized.

Show comment
Hide comment
@MarkPieszak

MarkPieszak Nov 4, 2017

Member

Was this merged in elsewhere ? @gkalpak

Member

MarkPieszak commented Nov 4, 2017

Was this merged in elsewhere ? @gkalpak

@IgorMinar

This comment has been minimized.

Show comment
Hide comment
Member

IgorMinar commented Nov 4, 2017

@jponeill

This comment has been minimized.

Show comment
Hide comment
@jponeill

jponeill Dec 6, 2017

Should this be reopened or is there some other way to see progress on this feature? (I checked the lab branch and it doesn't look like anything has changed since the backed out merge.)

jponeill commented Dec 6, 2017

Should this be reopened or is there some other way to see progress on this feature? (I checked the lab branch and it doesn't look like anything has changed since the backed out merge.)

@eggp

This comment has been minimized.

Show comment
Hide comment
@eggp

eggp Dec 7, 2017

it is a live feature ? it will be in the future ? :) last commit in: https://github.com/angular/angular/tree/labs/elements oct.12 :(

eggp commented Dec 7, 2017

it is a live feature ? it will be in the future ? :) last commit in: https://github.com/angular/angular/tree/labs/elements oct.12 :(

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Dec 8, 2017

Member

Not sure what you mean by "live feature". It is not out of labs yet (so not part of the actual release) if that is what you mean. Things under labs are experimental and not guaranteed to make it into core (at least that's my understanding), but I am pretty confident that "Angular Components as Custom Elements" will definitely make it into core - one way or another.

In the meantime, we are investigating some internal core changes that will also improve ngElements (e.g. make it possible to reduce the bundle size of bundled components, etc). And we might soon try out ngElements on specific parts of angular.io. Docs are also on the roadmap not too far down the road 😃

Stay tuned! And feel free to play around with labs/elements and give us feedback ❤️
(Some useful resources for getting started can be found in #20859.)/sub>

Member

gkalpak commented Dec 8, 2017

Not sure what you mean by "live feature". It is not out of labs yet (so not part of the actual release) if that is what you mean. Things under labs are experimental and not guaranteed to make it into core (at least that's my understanding), but I am pretty confident that "Angular Components as Custom Elements" will definitely make it into core - one way or another.

In the meantime, we are investigating some internal core changes that will also improve ngElements (e.g. make it possible to reduce the bundle size of bundled components, etc). And we might soon try out ngElements on specific parts of angular.io. Docs are also on the roadmap not too far down the road 😃

Stay tuned! And feel free to play around with labs/elements and give us feedback ❤️
(Some useful resources for getting started can be found in #20859.)/sub>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment