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

feat(ecmascript): support dynamic tenary operator jsvalue #6498

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Nov 17, 2023

Closes PACK-1963

Reattempt #6452

Closes PACK-1983

Copy link

vercel bot commented Nov 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-svelte-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Nov 17, 2023 7:18pm
10 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-cra-web ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 7:18pm

Copy link
Contributor

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Nov 17, 2023

🟢 CI successful 🟢

Thanks

Copy link
Contributor

Linux Benchmark for 1b48f6c

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 21.64ms ± 0.82ms 21.60ms ± 0.85ms -0.20%
bench_hmr_to_eval/Turbopack CSR/1000 modules 21.21ms ± 0.83ms 21.21ms ± 0.84ms -0.01%
bench_startup/Turbopack CSR/1000 modules 1145.48ms ± 2.75ms 1157.04ms ± 5.86ms +1.01%

Copy link
Contributor

github-actions bot commented Nov 17, 2023

🟢 Turbopack Benchmark CI successful 🟢

Thanks

@@ -469,9 +469,11 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
if test.is_truthy() == Some(true) {
*value = take(cons);
true
} else {
} else if test.is_falsy() == Some(true) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the culprit for the benchmark regresssion;

previously skipping falsy branching per #6452 (comment), however in that case non-truhy dynamic (i.e rootElement === null) is also considered as statically falsy and takes alt directly.

@kwonoj kwonoj changed the title [WIP] [DONOTMERGE] feat(ecmascript): support dynamic tenary operator jsvalue feat(ecmascript): support dynamic tenary operator jsvalue Nov 17, 2023
@kwonoj kwonoj marked this pull request as ready for review November 17, 2023 19:31
@kwonoj kwonoj requested a review from a team as a code owner November 17, 2023 19:31
Copy link
Contributor

Linux Benchmark for decec71

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 22.79ms ± 0.98ms 22.76ms ± 0.94ms -0.11%
bench_hmr_to_eval/Turbopack CSR/1000 modules 22.16ms ± 0.90ms 22.31ms ± 0.92ms +0.70%
bench_startup/Turbopack CSR/1000 modules 1163.42ms ± 3.05ms 1165.89ms ± 5.54ms +0.21%

@sokra sokra merged commit c16e402 into main Nov 17, 2023
51 checks passed
@sokra sokra deleted the revert-6496-revert-6452-jsvalue-tenary branch November 17, 2023 19:45
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.

None yet

2 participants