Skip to content

Fix more arrow functions that are passed in as "constructors"#26795

Merged
erwinmombay merged 2 commits intoampproject:masterfrom
erwinmombay:fix-more-esm
Feb 14, 2020
Merged

Fix more arrow functions that are passed in as "constructors"#26795
erwinmombay merged 2 commits intoampproject:masterfrom
erwinmombay:fix-more-esm

Conversation

@erwinmombay
Copy link
Copy Markdown
Member

arrow functions should not be passed in as constructors as eventually they will be new'd up and cause an error

also filed against closure compiler team google/closure-compiler#3550

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Feb 14, 2020

Hey @jridgewell, these files were changed:

  • build-system/eslint-rules/no-arrow-on-register-functions.js

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Feb 14, 2020

This pull request introduces 1 alert when merging 9fe89ce into 882bc62 - view on LGTM.com

new alerts:

  • 1 for Expression has no effect

@erwinmombay erwinmombay requested a review from rsimha February 14, 2020 02:18
@erwinmombay erwinmombay force-pushed the fix-more-esm branch 3 times, most recently from d0ea3c9 to 1728bdd Compare February 14, 2020 08:56
Copy link
Copy Markdown
Contributor

@gmajoulet gmajoulet left a comment

Choose a reason for hiding this comment

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

Story files LGTM.

@erwinmombay
Copy link
Copy Markdown
Member Author

per @rsimha i've skipped over a bunch of tests that seem to be only failing in isolation, they already fail on master if you isolate then in a unit test run. will try and fix the issue in a subsequent PR

@erwinmombay erwinmombay merged commit d3a72ea into ampproject:master Feb 14, 2020
erwinmombay added a commit that referenced this pull request Feb 14, 2020
* change all arrow functions registered as factory functions to normal function expressions

* skip tests that fail in isolation. these tests already fail on master
robinvanopstal added a commit to jungvonmatt/amphtml that referenced this pull request Feb 17, 2020
* master: (181 commits)
  🏗 Ensure valid flag usage for `gulp` tasks (ampproject#26814)
  build-system: Fix autocomplete error response (ampproject#26824)
  application/json is ab allowed type for <script> (ampproject#26815)
  🚮 Removing amp-consent-v2 experiment logic (ampproject#26162)
  Fix more arrow functions that are passed in as "constructors" (ampproject#26795)
  Variable substitution tester  (ampproject#26695)
  Turn on restrict fullscreen canary (ampproject#26766)
  Mock variableService getMacros  (ampproject#26300)
  Sync from Google (ampproject#26805)
  Sync from Google (ampproject#26803)
  Move video_state macro (ampproject#26212)
  🚀 Move ads variables to amp-analytics (ampproject#25113)
  Sync from Google (ampproject#26800)
  Sync from Google (ampproject#26798)
  Sync from Google (ampproject#26792)
  Another set of example.com change (ampproject#26753)
  Add PWA multidoc loader to examples (ampproject#26680)
  🐛Check for window null before creating tracking pixel (ampproject#26749)
  trying to update Sauce timeouts (ampproject#26737)
  🐛Fixes swipe to dismiss badly ordered swipes on `amp-lightbox-gallery` (ampproject#26788)
  ...

# Conflicts:
#	extensions/amp-accordion/amp-accordion.md
#	extensions/amp-bind/amp-bind.md
erwinmombay added a commit that referenced this pull request Feb 18, 2020
* change all arrow functions registered as factory functions to normal function expressions

* skip tests that fail in isolation. these tests already fail on master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants