-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
ref: Sort tokens in SourceMap #91
Conversation
**Description:** getsentry/rust-sourcemap#91 should fix this issue, but let's revert #9052 for now. # Context `swc_core` regressed. Caught by vercel/next.js#66902 ``` ⚠ Linting is disabled. ▲ Next.js 15.0.0-canary.34 ✓ Checking validity of types Creating an optimized production build ... Panic: PanicInfo { payload: Any { .. }, message: Some(attempt to add with overflow), location: Location { file: "/Users/kdy1/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sourcemap-8.0.1/src/encoder.rs", line: 89, col: 13 }, can_unwind: true, force_no_backtrace: false } Backtrace: 0: backtrace::backtrace::libunwind::trace at /Users/kdy1/.cargo/registry/src/index.crates.io-6f17d22bba15001f/backtrace-0.3.68/src/backtrace/libunwind.rs:93:5 backtrace::backtrace::trace_unsynchronized::<<backtrace::capture::Backtrace>::create::{closure#0}> at /Users/kdy1/.cargo/registry/src/index.crates.io-6f17d22bba15001f/backtrace-0.3.68/src/backtrace/mod.rs:66:5 backtrace::backtrace::trace::<<backtrace::capture::Backtrace>::create::{closure#0}> at /Users/kdy1/.cargo/registry/src/index.crates.io-6f17d22bba15001f/backtrace-0.3.68/src/backtrace/mod.rs:53:14 <backtrace::capture::Backtrace>::create at /Users/kdy1/.cargo/registry/src/index.crates.io-6f17d22bba15001f/backtrace-0.3.68/src/capture.rs:176:9 <backtrace::capture::Backtrace>::new at /Users/kdy1/.cargo/registry/src/index.crates.io-6f17d22bba15001f/backtrace-0.3.68/src/capture.rs:140:22 1: next_swc_napi::init::{closure#0} at packages/next-swc/crates/napi/src/lib.rs:85:29 2: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call at /rustc/6f3df08aadf71e8d4bf7e49f5dc10dfa6f254cb4/library/alloc/src/boxed.rs:2077:9 std::panicking::rust_panic_with_hook at /rustc/6f3df08aadf71e8d4bf7e49f5dc10dfa6f254cb4/library/std/src/panicking.rs:799:13 3: std::panicking::begin_panic_handler::{{closure}} at /rustc/6f3df08aadf71e8d4bf7e49f5dc10dfa6f254cb4/library/std/src/panicking.rs ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. you still keep the index
around in some places. does that really make sense to still expose it in some way? if it still exists internally, you might as well keep the APIs around for backwards compatibility.
That's unintentional then, I'll have to take another look. |
Can we have this merged/publsihed? |
Sorry, I lost track of this for a bit. Will give it a once-over and merge it soon. |
@Swatinem I just looked over this again; I don't think there's any |
Yeah, I also lost context by now. Anyway, I approved the PR back then, so just merge and profit :-) |
Thank you! |
@kdy1 It's released. |
The fact that the tokens in a sourcemap can be arbitrarily ordered causes a substantial amount of complication (we have to keep a sorted index in addition) for no benefit that I've ever seen. Therefore, we now sort tokens upon creation and remove all the
Index
rigmarole. This is a breaking change because it removes some types and functions.