AMP Runtime: allow usage of iterator/destructure/spread/rest

[samouri]

Description

Awhile back we decided to start allowing for iterator to be used (#34249). The logic was that it only adds 0.12kb to any bundle which uses it, and only .js builds as opposed to .mjs builds. We started implementation of this change, but ran into roadblocks (#34282 (review))

To do:

Update these plugins/rules to handle ArrayExpression:

• build-system/eslint-rules/no-deep-destructuring.js: eslint rule: no-deep-destructuring should handle ArrayPattern #35698
• build-system/eslint-rules/unused-private-field.js
• [Descoped] build-system/eslint-rules/prefer-destructuring.js

Can be removed:

• build-system/babel-plugins/babel-plugin-transform-simple-array-destructure/index.js (babel-plugins: remove simple-array-destructuring #35695)
• no-array-destructuring, no-rest, no-spread (lint: allow rest,spread, and array destructure #35699)

cc @ampproject/wg-performance

[samouri]

After local testing I believe:

• prefer-destructuring is optional and we can update at our leisure
• no-unused-privates does not need any updating, and already works through the ClassBody MemberExpression visitor

[jridgewell]

no-unused-privates does not need any updating, and already works through the ClassBody MemberExpression visitor

This won't catch uses like [this.#foo] = [1]. Honestly, we should just ban any patterns that don't use Identifier.

[samouri]

no-unused-privates does not need any updating, and already works through the ClassBody MemberExpression visitor

This won't catch uses like [this.#foo] = [1]. Honestly, we should just ban any patterns that don't use Identifier.

Sure. That sounds like a new eslint rule. Also I've never seen anyone use destructure to assign to a property before...its terrifying

[samouri]

no-unused-privates does not need any updating, and already works through the ClassBody MemberExpression visitor

This won't catch uses like [this.#foo] = [1]. Honestly, we should just ban any patterns that don't use Identifier.

On closer inspection, doesn't it still work? The case you just highlighted is about using an ArrayPattern to assign to a property. I think we'd want to throw when only seeing a private only used as an lvalue

[jridgewell]

The case you just highlighted is about using an ArrayPattern to assign to a property.

It doesn't detect that it's being written, it thinks it's being read. Which will incorrectly flag the field as being used.

[samouri]

I'm going to descope prefer-destructuring as a nice-to-have, and say once unused-private-field is updated that this issue can be closed.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.