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

fix(es/compat): Visit transformed expression in optional chaining pass #7614

Merged
merged 13 commits into from
Jul 4, 2023

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Jul 3, 2023

Description:

Related issue:

@kdy1 kdy1 added this to the Planned milestone Jul 3, 2023
@kdy1 kdy1 self-assigned this Jul 3, 2023
@kdy1 kdy1 requested a review from jridgewell July 3, 2023 02:49
@kdy1 kdy1 marked this pull request as ready for review July 3, 2023 02:49
Copy link
Member Author

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

swc-bump:

  • swc_ecma_transforms_compat

kodiakhq[bot]
kodiakhq bot previously approved these changes Jul 3, 2023
kodiakhq[bot]
kodiakhq bot previously approved these changes Jul 3, 2023
Comment on lines 1 to 16
const ModalInner = ({ data }) => {
return (
<Album
data={(qcData?.flawImageList || [])?.map(src => ({
url: src,
describe: qcData?.flawDesc
}))}
showIndicators
useSafeAreaBottom
showDescription
current={current}
>
test
</Album>
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this is enough to trigger the bug:

Suggested change
const ModalInner = ({ data }) => {
return (
<Album
data={(qcData?.flawImageList || [])?.map(src => ({
url: src,
describe: qcData?.flawDesc
}))}
showIndicators
useSafeAreaBottom
showDescription
current={current}
>
test
</Album>
)
}
(qcData?.flawImageList || [])?.map()

@@ -64,6 +64,7 @@ impl VisitMut for OptChaining {
Expr::OptChain(v) => {
let data = self.gather(v.take(), vec![]);
*e = self.construct(data, false, self.c.no_document_all);
e.visit_mut_children_with(self);
Copy link
Contributor

Choose a reason for hiding this comment

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

We would need the same call for Expr::Unary, but I think the better solution is to fix it at https://github.com/kdy1/swc/blob/9f44d10bb2a28aa7d776c2a4243074e8f59494f7/crates/swc_ecma_transforms_compat/src/es2020/optional_chaining.rs#L207. That'll avoid a repeated iteration of prop and args (we do them during gathering). The only thing I missed was transforming the base

@@ -8,4 +8,4 @@ var _foo, _a_b_c, _a, _a_b_c_d, _a_b, _a_b_c_d1, _a_b_c1, _orders, _orders_, _or
orders[(_client = client) === null || _client === void 0 ? void 0 : _client.key].price;
(_orders_client_key = orders[client.key]) === null || _orders_client_key === void 0 ? void 0 : _orders_client_key.price;
(0, (_a1 = a) === null || _a1 === void 0 ? void 0 : _a1.b).c;
(0, (_c = (0, a?.b).c) === null || _c === void 0 ? void 0 : _c.d).e;
Copy link
Contributor

Choose a reason for hiding this comment

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

🤦

kodiakhq[bot]
kodiakhq bot previously approved these changes Jul 4, 2023
kodiakhq[bot]
kodiakhq bot previously approved these changes Jul 4, 2023
@kdy1 kdy1 requested a review from jridgewell July 4, 2023 04:46
jridgewell
jridgewell previously approved these changes Jul 4, 2023
@kdy1 kdy1 enabled auto-merge (squash) July 4, 2023 05:35
@kdy1 kdy1 dismissed stale reviews from jridgewell and kodiakhq via b65de5b July 4, 2023 05:35
Copy link
Collaborator

@swc-bot swc-bot left a comment

Choose a reason for hiding this comment

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

Automated review comment generated by auto-rebase script

@kdy1 kdy1 merged commit c087c82 into swc-project:main Jul 4, 2023
249 checks passed
@kdy1 kdy1 deleted the issue-7612 branch July 4, 2023 07:00
@kdy1 kdy1 modified the milestones: Planned, v1.3.68 Jul 5, 2023
@swc-project swc-project locked as resolved and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

optional chaining syntax is not compiled
3 participants