Open loewenheim opened 1 year ago
The crate is also implementing binary_search
manually, as well as its own version of IndexMap
.
There is also unsafe code related to SourceView
, which is a type that definitely has nothing to do in a generic SourceMap
crate. Similar to how the heuristics-based "find original function name" is very out of place.
The problem with the above is that it is still used from the legacy Sentry processor via symbolic
. I would like to kill all that with fire, but I’m unsure about the right time to do so.
I’m also a bit unsure about making all the fields public. The crate already has a distinction between a raw JSON sourcemap, vs a more concrete type that you can build and manipulate actively.
It might make sense to keep that distinction, but there is an argument to be made to make the concrete type more opaque. But at the same time, having different variants for SourceMapHermes
for example does make sense, as a sourcemap with the hermes
-style extension for scopes does serve a different usecase.
This crate is pretty old and has a number of outdated/unidiomatic features. For example:
SourceMapBuilder::into_sourcemap
should be calledbuild
.SourceMap
stores tokens as an unordered vector with a separate ordered index. There is no clear reason tokens can't be stored in an ordered way in the first place (as a vector or BTreeMap).SourceMap
stores sources as aVec<String>
, but sources may benull
in the sourcemap JSON format. This leads tonull
sources being replaced with""
during parsing, to unclear benefit.SourceMap
has a lot of getter and setter methods that could just as well be replaced with mutable access to the corresponding fields.