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(ivy): make Hammer support tree-shakable #32203

Closed
wants to merge 1 commit into from

Conversation

@kara
Copy link
Contributor

commented Aug 20, 2019

Currently, it's not possible to tree-shake away the
coordination layer between HammerJS and Angular's
EventManager. This means that you get the HammerJS
support code in your production bundle whether or
not you actually use the library.

This commit removes the Hammer providers from the
default platform_browser providers list and instead
provides them as part of a HammerModule. Apps on
Ivy just need to import the HammerModule at root
to turn on Hammer support. Otherwise all Hammer code
will tree-shake away. View Engine apps will require
no change.

Note: MatSlideToggle and MatSlider tests are disabled
until Material can make a change on that side.

@kara kara requested a review from angular/fw-core as a code owner Aug 20, 2019

@googlebot googlebot added the cla: yes label Aug 20, 2019

@ngbot ngbot bot added this to the needsTriage milestone Aug 20, 2019

@IgorMinar
Copy link
Member

left a comment

The ci is unhappy but at the high level this lgtm.

Please add a breaking change note to the commit message.

@kara kara force-pushed the kara:hammer branch from a10bc8c to 9fcfd50 Aug 20, 2019

@kara

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2019

@pkozlowski-opensource
Copy link
Member

left a comment

LGTM (minus CI being red, of course), like the direction a lot!
Out of curiosity: do you know how much it saves for an application that is not using gestures?

@kara kara force-pushed the kara:hammer branch from 9fcfd50 to 8e04ba4 Aug 20, 2019

@kara

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2019

@pkozlowski-opensource It should save 2-3 KB

@kara kara force-pushed the kara:hammer branch from 8e04ba4 to ae3052a Aug 20, 2019

@kara kara requested review from angular/fw-integration as code owners Aug 20, 2019

kara added a commit to kara/material2 that referenced this pull request Aug 20, 2019
ci: temporarily disable gesture config tests
We need to temporarily disable the gesture tests in order to
land a PR in framework that makes Hammer support optional
(angular/angular#32203). Otherwise,
the Material/FW integration tests will fail because Material
modules haven't yet opted into Hammer.

Unfortunately, we cannot yet fix this properly by adding the
HammerModule to Material because Material is not using a
release that has HammerModule. Once the PR lands and Material
updates to a 9.0.0-next release, we can add the HammerModule
properly and turn these tests back on.
kara added a commit to angular/components that referenced this pull request Aug 20, 2019
ci: temporarily disable gesture config tests
We need to temporarily disable the gesture tests in order to
land a PR in framework that makes Hammer support optional
(angular/angular#32203). Otherwise,
the Material/FW integration tests will fail because Material
modules haven't yet opted into Hammer.

Unfortunately, we cannot yet fix this properly by adding the
HammerModule to Material because Material is not using a
release that has HammerModule. Once the PR lands and Material
updates to a 9.0.0-next release, we can add the HammerModule
properly and turn these tests back on.
kara added a commit to angular/components that referenced this pull request Aug 20, 2019
ci: temporarily disable gesture config tests
We need to temporarily disable the gesture tests in order to
land a PR in framework that makes Hammer support optional
(angular/angular#32203). Otherwise,
the Material/FW integration tests will fail because Material
modules haven't yet opted into Hammer.

Unfortunately, we cannot yet fix this properly by adding the
HammerModule to Material because Material is not using a
release that has HammerModule. Once the PR lands and Material
updates to a 9.0.0-next release, we can add the HammerModule
properly and turn these tests back on.
@kara

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2019

jelbourn added a commit to angular/components that referenced this pull request Aug 20, 2019
ci: temporarily disable gesture config tests (#16828)
We need to temporarily disable the gesture tests in order to
land a PR in framework that makes Hammer support optional
(angular/angular#32203). Otherwise,
the Material/FW integration tests will fail because Material
modules haven't yet opted into Hammer.

Unfortunately, we cannot yet fix this properly by adding the
HammerModule to Material because Material is not using a
release that has HammerModule. Once the PR lands and Material
updates to a 9.0.0-next release, we can add the HammerModule
properly and turn these tests back on.

@kara kara removed the PR state: blocked label Aug 20, 2019

@kara kara force-pushed the kara:hammer branch from ae3052a to eae8751 Aug 20, 2019

@kara kara force-pushed the kara:hammer branch from eae8751 to e63d966 Aug 20, 2019

@kara kara removed the PR state: blocked label Aug 20, 2019

@kara kara force-pushed the kara:hammer branch from e63d966 to 362ca05 Aug 21, 2019

@kara kara removed the PR state: blocked label Aug 21, 2019

@kara kara force-pushed the kara:hammer branch from 362ca05 to 0db9214 Aug 21, 2019

feat(ivy): make Hammer support tree-shakable
Currently, it's not possible to tree-shake away the
coordination layer between HammerJS and Angular's
EventManager. This means that you get the HammerJS
support code in your production bundle whether or
not you actually use the library.

This commit removes the Hammer providers from the
default platform_browser providers list and instead
provides them as part of a `HammerModule`. Apps on
Ivy just need to import the `HammerModule` at root
to turn on Hammer support. Otherwise all Hammer code
will tree-shake away. View Engine apps will require
no change.

BREAKING CHANGE

Previously, in Ivy applications, Hammer providers
were included by default. With this commit, apps
that want Hammer support must import `HammerModule`
in their root module.

@kara kara force-pushed the kara:hammer branch from 0db9214 to aa322d1 Aug 21, 2019

andrewseguin added a commit to angular/components that referenced this pull request Aug 26, 2019
ci: temporarily disable gesture config tests (#16828)
We need to temporarily disable the gesture tests in order to
land a PR in framework that makes Hammer support optional
(angular/angular#32203). Otherwise,
the Material/FW integration tests will fail because Material
modules haven't yet opted into Hammer.

Unfortunately, we cannot yet fix this properly by adding the
HammerModule to Material because Material is not using a
release that has HammerModule. Once the PR lands and Material
updates to a 9.0.0-next release, we can add the HammerModule
properly and turn these tests back on.

(cherry picked from commit eaf70ca)
ngdevelop-tech added a commit to ngdevelop-tech/angular that referenced this pull request Aug 27, 2019
feat(ivy): make Hammer support tree-shakable (angular#32203)
Currently, it's not possible to tree-shake away the
coordination layer between HammerJS and Angular's
EventManager. This means that you get the HammerJS
support code in your production bundle whether or
not you actually use the library.

This commit removes the Hammer providers from the
default platform_browser providers list and instead
provides them as part of a `HammerModule`. Apps on
Ivy just need to import the `HammerModule` at root
to turn on Hammer support. Otherwise all Hammer code
will tree-shake away. View Engine apps will require
no change.

BREAKING CHANGE

Previously, in Ivy applications, Hammer providers
were included by default. With this commit, apps
that want Hammer support must import `HammerModule`
in their root module.

PR Close angular#32203
sabeersulaiman added a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
feat(ivy): make Hammer support tree-shakable (angular#32203)
Currently, it's not possible to tree-shake away the
coordination layer between HammerJS and Angular's
EventManager. This means that you get the HammerJS
support code in your production bundle whether or
not you actually use the library.

This commit removes the Hammer providers from the
default platform_browser providers list and instead
provides them as part of a `HammerModule`. Apps on
Ivy just need to import the `HammerModule` at root
to turn on Hammer support. Otherwise all Hammer code
will tree-shake away. View Engine apps will require
no change.

BREAKING CHANGE

Previously, in Ivy applications, Hammer providers
were included by default. With this commit, apps
that want Hammer support must import `HammerModule`
in their root module.

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