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

Make stringify_str and stringify_module_id use Serialize #3770

Merged
merged 2 commits into from Feb 13, 2023

Conversation

alexkirsz
Copy link
Contributor

@alexkirsz alexkirsz commented Feb 13, 2023

Both stringify_str and stringify_module_id are now simply stringify_js. We could skip this and go directly to serde_json, but this hides the dependency behind turbopack-ecmascript.

@vercel
Copy link

vercel bot commented Feb 13, 2023

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

Name Status Preview Comments Updated
examples-cra-web 🔄 Building (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-designsystem-docs 🔄 Building (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-svelte-web 🔄 Building (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-vite-web 🔄 Building (Inspect) Feb 13, 2023 at 4:27PM (UTC)
6 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 13, 2023 at 4:27PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Feb 13, 2023 at 4:27PM (UTC)
turbo-site ⬜️ Ignored (Inspect) Visit Preview Feb 13, 2023 at 4:27PM (UTC)

@alexkirsz
Copy link
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 13, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

crates/turbopack-core/src/chunk/mod.rs Outdated Show resolved Hide resolved
@alexkirsz alexkirsz added the pr: automerge Kodiak will merge these automatically after checks pass label Feb 13, 2023
@kodiakhq kodiakhq bot merged commit 10686b1 into main Feb 13, 2023
@kodiakhq kodiakhq bot deleted the alexkirsz/web-455-stringify-js branch February 13, 2023 16:51
@github-actions
Copy link
Contributor

Benchmark for cb33eac

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9465.82µs ± 70.29µs 9439.26µs ± 64.56µs -0.28%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.25ms ± 0.37ms 12.46ms ± 0.28ms +1.71%
bench_hmr_to_commit/Turbopack RSC/1000 modules 503.27ms ± 1.67ms 502.13ms ± 1.39ms -0.23%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9497.21µs ± 57.47µs 9496.20µs ± 81.16µs -0.01%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8493.29µs ± 91.21µs 8492.04µs ± 50.08µs -0.01%
bench_hmr_to_eval/Turbopack RCC/1000 modules 10.60ms ± 0.22ms 10.47ms ± 0.25ms -1.19%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8531.01µs ± 47.21µs 8500.89µs ± 62.26µs -0.35%
bench_hydration/Turbopack RCC/1000 modules 4185.29ms ± 10.70ms 4169.96ms ± 9.01ms -0.37%
bench_hydration/Turbopack RSC/1000 modules 3790.14ms ± 15.33ms 3776.48ms ± 7.76ms -0.36%
bench_hydration/Turbopack SSR/1000 modules 3639.40ms ± 25.16ms 3662.40ms ± 14.34ms +0.63%
bench_startup/Turbopack CSR/1000 modules 2773.87ms ± 9.29ms 2777.59ms ± 8.13ms +0.13%
bench_startup/Turbopack RCC/1000 modules 2535.28ms ± 6.74ms 2548.27ms ± 5.36ms +0.51%
bench_startup/Turbopack RSC/1000 modules 2427.62ms ± 9.23ms 2445.01ms ± 10.54ms +0.72%
bench_startup/Turbopack SSR/1000 modules 2112.06ms ± 2.65ms 2103.87ms ± 2.52ms -0.39%

jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
…o#3770)

Both `stringify_str` and `stringify_module_id` are now simply `stringify_js`. We could skip this and go directly to `serde_json`, but this hides the dependency behind turbopack-ecmascript.
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
…o#3770)

Both `stringify_str` and `stringify_module_id` are now simply `stringify_js`. We could skip this and go directly to `serde_json`, but this hides the dependency behind turbopack-ecmascript.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants