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
wants to merge 2 commits into from
Closed

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

wants to merge 2 commits into from

Conversation

alan-agius4
Copy link
Contributor

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 added the area: core Issues related to the framework runtime label Apr 10, 2019
@alan-agius4 alan-agius4 requested review from a team as code owners April 10, 2019 11:09
@ngbot ngbot bot added this to the needsTriage milestone Apr 10, 2019
@alan-agius4 alan-agius4 added the target: major This PR is targeted for the next major release label Apr 10, 2019
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
Copy link
Contributor

@mhevery mhevery left a comment

Choose a reason for hiding this comment

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

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

@mhevery mhevery self-assigned this Apr 11, 2019
@alan-agius4
Copy link
Contributor Author

@mhevery agreed, added.

@mhevery mhevery added the action: merge The PR is ready for merge by the caretaker label Apr 17, 2019
@mhevery
Copy link
Contributor

mhevery commented Apr 17, 2019

@benlesh benlesh closed this in 4bde40f Apr 17, 2019
@alan-agius4 alan-agius4 deleted the core_dts_event_listener branch April 18, 2019 04:04
Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

lgtm

BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
…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
@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 Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core.d.ts exports its own EventListener (dts bundling issue with 8.0.0-beta.8)
5 participants