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

fix(core): don't include a local `EventListener` in typings #29809

Closed

Conversation

@alan-agius4
Copy link
Contributor

commented Apr 10, 2019

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:

With dts bundles, core.d.ts will include an EventListener class as it's used in

readonly listeners: EventListener[];

This will conflict with the DOM EventListener, as anything in core.d.ts which is using the DOM EventListener will fallback in using the one defined in the same module and hence build will fail because their implementation is different.

With this change, we rename the local EventListener to DebugEventListener, the later one is non exported.

Fixes #29806

Does this PR introduce a breaking change?

  • Yes
  • [x ] No

Other information

@alan-agius4 alan-agius4 requested review from angular/fw-core as code owners Apr 10, 2019

@ngbot ngbot bot added this to the needsTriage milestone Apr 10, 2019

@googlebot googlebot added the cla: yes label Apr 10, 2019

@alan-agius4 alan-agius4 force-pushed the alan-agius4:core_dts_event_listener branch from 7557656 to 58271b6 Apr 10, 2019

fix(core): don't include a local `EventListener` in typings
With dts bundles, `core.d.ts` will include an `EventListener` class as it's used in https://github.com/angular/angular/blob/303eae918d997070a36b523ddc97e018f622c258/packages/core/src/debug/debug_node.ts#L32

This will conflict with the DOM EventListener, as anything in `core.d.ts` which is using the DOM EventListener will fallback in using the one defined in the same module and hence build will fail because their implementation is different.

With this change, we rename the local `EventListener` to `DebugEventListener`, the later one is non exported.

Fixes #29806

@alan-agius4 alan-agius4 changed the title fix(core): remove `EventListener` from it's exports fix(core): don't include a local `EventListener` in typings Apr 10, 2019

@alan-agius4 alan-agius4 force-pushed the alan-agius4:core_dts_event_listener branch from 58271b6 to 90fa5d1 Apr 10, 2019

@mhevery
Copy link
Member

left a comment

I think DebugEventListener should be exported as well. Could you add it?

@mhevery mhevery self-assigned this Apr 11, 2019

@alan-agius4 alan-agius4 requested a review from mhevery Apr 11, 2019

@alan-agius4

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

@mhevery agreed, added.

@mhevery

This comment has been minimized.

@benlesh benlesh closed this in 4bde40f Apr 17, 2019

@alan-agius4 alan-agius4 deleted the alan-agius4:core_dts_event_listener branch Apr 18, 2019

@IgorMinar
Copy link
Member

left a comment

lgtm

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

fix(core): don't include a local `EventListener` in typings (angular#…
…29809)

With dts bundles, `core.d.ts` will include an `EventListener` class as it's used in https://github.com/angular/angular/blob/303eae918d997070a36b523ddc97e018f622c258/packages/core/src/debug/debug_node.ts#L32

This will conflict with the DOM EventListener, as anything in `core.d.ts` which is using the DOM EventListener will fallback in using the one defined in the same module and hence build will fail because their implementation is different.

With this change, we rename the local `EventListener` to `DebugEventListener`, the later one is non exported.

Fixes angular#29806

PR Close angular#29809
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.