Skip to content

feat(#54): add outputs with types and docs to angular proxies#55

Merged
jthoms1 merged 3 commits intostenciljs:masterfrom
razvangeangu:master
Jun 24, 2020
Merged

feat(#54): add outputs with types and docs to angular proxies#55
jthoms1 merged 3 commits intostenciljs:masterfrom
razvangeangu:master

Conversation

@razvangeangu
Copy link
Copy Markdown
Contributor

@razvangeangu razvangeangu commented Jun 17, 2020

The outputs from the angular proxies should inherit the events from the stencil component class.

The current implementation adds the outputs as the properties of the angular class.

import { Components } from 'component-library'
export declare interface MyButton extends Components.MyButton {}
@ProxyCmp({ inputs: ["buttonType"] })
@Component({
  selector: "my-button",
  changeDetection: ChangeDetectionStrategy.OnPush,
  template: "<ng-content></ng-content>",
  inputs: ["buttonType"],
})
export class MyButton {
  ionFocus!: EventEmitter<CustomEvent>;
  protected el: HTMLElement;
  constructor(c: ChangeDetectorRef, r: ElementRef, protected z: NgZone) {
    c.detach();
    this.el = r.nativeElement;
    proxyOutputs(this, this.el, ["ionFocus"]);
  }
}

I believe they should be inherited from the stencil class like this to inherit types and docs in similar fashion to how inputs are added.

import { Components } from 'component-library'
++ import { Button as IButton } from 'component-library/dist/types/components/my-button/my-button'
++ export declare interface MyButton extends Required<Pick<IButton, 'ionFocus'>> {}
export declare interface MyButton extends Components.MyButton {}
@ProxyCmp({ inputs: ["buttonType"] })
@Component({
  selector: "my-button",
  changeDetection: ChangeDetectionStrategy.OnPush,
  template: "<ng-content></ng-content>",
  inputs: ["buttonType"],
++  outputs: ["ionFocus"]
})
export class MyButton {
--  ionFocus!: EventEmitter<CustomEvent>;
  protected el: HTMLElement;
  constructor(c: ChangeDetectorRef, r: ElementRef, protected z: NgZone) {
    c.detach();
    this.el = r.nativeElement;
    proxyOutputs(this, this.el, ["ionFocus"]);
  }
}

directiveOpts.push(`inputs: ['${inputs.join(`', '`)}']`);
}
if (outputs.length > 0) {
directiveOpts.push(`outputs: ['${outputs.join(`', '`)}']`)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is to inform the angular html parser about the available outputs.


const outputsInterface =
outputs.length > 0
? `export declare interface ${tagNameAsPascal} extends Required<Pick<JSX.${tagNameAsPascal}, '${outputs.map(o => 'on' + o.replace(/^./, match => match.toUpperCase())).join(`' | '`).replace(/\| $/g, '')}'>> {}`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here, the only available import I could find is JSX.tagNameAsPascal. Ideally this should come from dist/types/components/component/component.d.ts. That way, we can get rid of adding the on prefix and correctly inherit the type EventEmitter.

Pick - will take only the events mentioned in the union
Required - will mark the selected events as required to match the previous implementation.

Copy link
Copy Markdown
Contributor Author

@razvangeangu razvangeangu Jun 17, 2020

Choose a reason for hiding this comment

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

I can see how custom names for events would be a problem so I moved from this strategy to the following:

import { Components } from 'component-library';
import { Button as IButton } from 'component-library/dist/types/components/my-button/my-button'
-- export declare interface MyButton extends Required<Pick<IButton, 'ionFocus'>> {}
export declare interface MyButton extends Components.MyButton {}
@ProxyCmp({ inputs: ["buttonType"] })
@Component({
  selector: "my-button",
  changeDetection: ChangeDetectionStrategy.OnPush,
  template: "<ng-content></ng-content>",
  inputs: ["buttonType"],
  outputs: ["ionFocus"]
})
export class MyButton {
++  /** Emitted after the popover has presented. */
++  ionButtonFocus!: IButton['ionFocus'];
  constructor(c: ChangeDetectorRef, r: ElementRef, protected z: NgZone) {
    c.detach();
    this.el = r.nativeElement;
    proxyOutputs(this, this.el, ["ionFocus"]);
  }
}

output.name gets typeof output.method

@razvangeangu
Copy link
Copy Markdown
Contributor Author

I can see the js and the original ts source absolute paths but I cannot seem to find an easy way to to get the type path.

const outputsInterface =
outputs.length > 0
? `export declare interface ${tagNameAsPascal} extends Required<Pick<JSX.${tagNameAsPascal}, '${outputs.map(o => 'on' + o.replace(/^./, match => match.toUpperCase())).join(`' | '`).replace(/\| $/g, '')}'>> {}`
? `import { ${cmpMeta.componentClassName} as I${cmpMeta.componentClassName} } from '${typePath}';`
Copy link
Copy Markdown
Contributor Author

@razvangeangu razvangeangu Jun 17, 2020

Choose a reason for hiding this comment

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

I guess this makes much more sense - however, build is failing due to unresolved types that exist in the global scope of the component-library (i.e. interfaces.d.ts). Suggestions?

const tagNameAsPascal = dashToPascalCase(cmpMeta.tagName);
const lines = [`

const typePath = path.relative(componentCorePackage, cmpMeta.sourceFilePath).replace('../src', path.join(componentCorePackage, distTypesDir)).replace('.tsx', '');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

replace('../src' and '.tsx') need to go.. Suggestions?

@jthoms1 jthoms1 self-assigned this Jun 23, 2020
@jthoms1 jthoms1 merged commit dd95b4b into stenciljs:master Jun 24, 2020
@jthoms1
Copy link
Copy Markdown
Contributor

jthoms1 commented Jun 24, 2020

Thanks for the PR!

@jthoms1
Copy link
Copy Markdown
Contributor

jthoms1 commented Jun 24, 2020

closes #54

@jthoms1 jthoms1 added the type: feature New feature, request or improvement. label Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature New feature, request or improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants