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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃彈 Add a default assignment aliasing transform #30250

Merged
merged 4 commits into from
Sep 25, 2020

Conversation

jridgewell
Copy link
Contributor

Fixes #29293

@amp-owners-bot
Copy link

amp-owners-bot bot commented Sep 16, 2020

Hey @erwinmombay! These files were changed:

build-system/babel-plugins/babel-plugin-transform-default-assignment/index.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/should-transform-param-destructures/input.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/should-transform-param-destructures/options.json
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/should-transform-param-destructures/output.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/should-transform-param/input.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/should-transform-param/options.json
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/should-transform-param/output.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/throws-on-param-assignment-with-destructure/input.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/throws-on-param-assignment-with-destructure/options.json
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/throws-when-default-destructure-references-default-destructure/input.js
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/throws-when-default-destructure-references-default-destructure/options.json
build-system/babel-plugins/babel-plugin-transform-default-assignment/test/fixtures/transform-assertions/throws-when-default-destructure-references-default/input.js
+6 more

@@ -243,7 +243,8 @@ class AmpCarousel extends AMP.BaseElement {
* actionSource: (!ActionSource|undefined),
* }=} options
*/
goToSlide(index, {smoothScroll = false, actionSource} = {}) {
goToSlide(index, options = {}) {
const {smoothScroll = false, actionSource} = options;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify why this again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The @param is named, and it's too difficult to figure out what that is when creating the binding. If it's not named properly, we closure will complain throw.

@jridgewell jridgewell merged commit 0c5ca56 into ampproject:master Sep 25, 2020
@jridgewell jridgewell deleted the default-assignment-transform branch September 25, 2020 19:51
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
* Add a default assignment aliasing transform

Fixes ampproject#29293

* Only transform parameter default assignments

* Lint

* Lint
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.

Closure and prop types: default values are broken when desctructuring
3 participants