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

馃毊 Remove transform-simple-object-destructure #27689

Conversation

jridgewell
Copy link
Contributor

It's no longer needed for check-types.

I also did a drive-by standardizing our babel test configs.

Fixes #27119.

@amp-owners-bot
Copy link

Hey @erwinmombay, these files were changed:

build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/no-transform/options.json
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/nomodule/options.json
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/transform/options.json
build-system/babel-plugins/babel-plugin-const-transformer/test/fixtures/transform-assertions/ignored-let/options.json
build-system/babel-plugins/babel-plugin-const-transformer/test/fixtures/transform-assertions/ignored-var/options.json
build-system/babel-plugins/babel-plugin-const-transformer/test/fixtures/transform-assertions/should-transform/options.json
build-system/babel-plugins/babel-plugin-is_dev-constant-transformer/test/fixtures/transform-assertions/isdev-transform/options.json
build-system/babel-plugins/babel-plugin-is_dev-constant-transformer/test/fixtures/transform-assertions/no-transform/options.json
build-system/babel-plugins/babel-plugin-is_minified-constant-transformer/test/fixtures/transform-assertions/isminified-transform/options.json
build-system/babel-plugins/babel-plugin-is_minified-constant-transformer/test/fixtures/transform-assertions/no-transform/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/no-transform-user-assert/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/no-transform-userAssert/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/no-transform/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/preserve-parens/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean-non/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-element/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number-non/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string-not-string-non/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-empty-string/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-false/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-null/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-zero/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert/options.json
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-user-fine/options.json
build-system/babel-plugins/babel-plugin-transform-amp-extension-call/test/fixtures/transform-call/transform-simple-body/options.json
build-system/babel-plugins/babel-plugin-transform-annotation-to-extern/test/fixtures/extract-annotations/emit-typedefs/options.json
build-system/babel-plugins/babel-plugin-transform-annotation-to-extern/test/fixtures/extract-annotations/remove-typedefs/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/argument-function-identifier/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/argument-function/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/arguments-identifier/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/async-function/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/class-function-declaration-in-public-method/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/existing-arrow-function/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/existing-async-arrow-function/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/export-default-named-declaration/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/export-named-declaration/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/function-expression/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/generator-function/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/multiple-body-statements/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/nested-function-inside-export/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/new-expression/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/no-return/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/single-return-statement/options.json
build-system/babel-plugins/babel-plugin-transform-function-declarations/test/fixtures/transform-assertions/this-argument/options.json
build-system/babel-plugins/babel-plugin-transform-html-template/test/fixtures/transform-assertions/direct/options.json
build-system/babel-plugins/babel-plugin-transform-html-template/test/fixtures/transform-assertions/indirect/options.json
build-system/babel-plugins/babel-plugin-transform-html-template/test/fixtures/transform-assertions/invalid/options.json
build-system/babel-plugins/babel-plugin-transform-html-template/test/fixtures/transform-assertions/other-template-literals/options.json
build-system/babel-plugins/babel-plugin-transform-json-configuration/test/fixtures/transform/escapes/options.json
build-system/babel-plugins/babel-plugin-transform-json-configuration/test/fixtures/transform/json-literal/options.json
build-system/babel-plugins/babel-plugin-transform-json-configuration/test/fixtures/transform/transform/options.json
build-system/babel-plugins/babel-plugin-transform-log-methods/test/fixtures/transform-log-methods/invalid/options.json
build-system/babel-plugins/babel-plugin-transform-log-methods/test/fixtures/transform-log-methods/no-args/options.json
build-system/babel-plugins/babel-plugin-transform-log-methods/test/fixtures/transform-log-methods/no-message/options.json
build-system/babel-plugins/babel-plugin-transform-log-methods/test/fixtures/transform-log-methods/one-arg-variadic/options.json
build-system/babel-plugins/babel-plugin-transform-log-methods/test/fixtures/transform-log-methods/template-literals/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/errors/computed-identifier-property/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/errors/computed-number-property/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/errors/computed-string-property/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/errors/deep-computed-identifier/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/unused-properties/double-assignment/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/unused-properties/other-namespace/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/unused-properties/underscore-namespace/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/unused-properties/using-other-namespace/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/used-properties/deep-access/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/used-properties/prototypes/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/used-properties/simple/options.json
build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/fixtures/used-properties/update-expression/options.json
build-system/babel-plugins/babel-plugin-transform-remove-directives/test/fixtures/transform/directives/options.json
build-system/babel-plugins/babel-plugin-transform-simple-array-destructure/test/fixtures/transform/deep-array/options.json
build-system/babel-plugins/babel-plugin-transform-simple-array-destructure/test/fixtures/transform/deep/options.json
build-system/babel-plugins/babel-plugin-transform-simple-array-destructure/test/fixtures/transform/param/options.json
build-system/babel-plugins/babel-plugin-transform-simple-array-destructure/test/fixtures/transform/skip-element/options.json
build-system/babel-plugins/babel-plugin-transform-simple-array-destructure/test/fixtures/transform/variable-declarations/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/index.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/no-transform/array-destructure/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/no-transform/array-destructure/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/no-transform/array-destructure/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/no-transform/param-destructure/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/no-transform/param-destructure/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/no-transform/param-destructure/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/computed-keys/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/computed-keys/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/computed-keys/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/default-values/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/default-values/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/default-values/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/non-identifier-keys/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/non-identifier-keys/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/non-identifier-keys/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/variable-declarations-rename/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/variable-declarations-rename/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/variable-declarations-rename/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/variable-declarations/input.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/variable-declarations/options.json
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/fixtures/transform/variable-declarations/output.js
build-system/babel-plugins/babel-plugin-transform-simple-object-destructure/test/index.js
build-system/babel-plugins/babel-plugin-transform-stringish-literals/test/fixtures/transform/binary-expression/options.json
build-system/babel-plugins/babel-plugin-transform-stringish-literals/test/fixtures/transform/literals/options.json
build-system/babel-plugins/babel-plugin-transform-version-call/test/fixtures/transform/transform-internal-runtime-version-call/options.json

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Change to build-system/babel-config/pre-closure-config.js LGTM. Will defer to @kristoferbaxter or @erwinmombay for the rest of the changes.

@kristoferbaxter
Copy link
Contributor

Looks like type checking is still failing. Can you also pull in latest master to ensure the ESM build passes?

Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Looks like typechecks do not work with this change still. There appears to be another issue.

@jridgewell jridgewell force-pushed the remove-transform-simple-object-destructure branch from 14656bc to 2c7c004 Compare April 10, 2020 23:38
@jridgewell
Copy link
Contributor Author

Rebased, and fixed the failing type.

Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Nice work!

@jridgewell jridgewell merged commit 1dcbc8d into ampproject:master Apr 13, 2020
@jridgewell jridgewell deleted the remove-transform-simple-object-destructure branch April 13, 2020 21:39
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.

Remove Simple Destructure Transform for Type Checks
5 participants