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

Non-async source map. #331

Open
ahmedcharles opened this issue Apr 24, 2018 · 29 comments
Open

Non-async source map. #331

ahmedcharles opened this issue Apr 24, 2018 · 29 comments
Labels
feat New feature

Comments

@ahmedcharles
Copy link

Is it possible to use the new Rust wasm lib-mapping without requiring support for async/await or futures, in general?

For context, I'm writing code in a context where all async operations are disallowed, which means that I'm stuck on 0.6.x of source-map. I was hoping to get the performance benefit of the wasm implementation but requiring async is a deal breaker.

@fitzgen
Copy link
Contributor

fitzgen commented Apr 25, 2018

Fetching and compiling WebAssembly is inherently async.

We could add a sync API to create a SourceMapConsumer given that you've already fetched and compiled the wasm. Would that work?

If so, I can help you craft a PR that implements this.

@ahmedcharles
Copy link
Author

I'm not sure. Does node require an async interface for wasm? For reference, I'd like to use source maps with screeps (0.6.x works). Their docs for wasm is here: http://docs.screeps.com/modules.html It doesn't seem to include anything async, but I've also not tested it yet.

@SimenB
Copy link
Contributor

SimenB commented May 2, 2018

A sync API is also needed for e.g. stack traces, as they are collected synchronously (evanw/node-source-map-support#206).

If we can initialize whatever needs to be async ahead of time, and have a sync way of constructing and using SourceMapConsumers, that would unblock Jest (and probably Babel) at least

@fitzgen
Copy link
Contributor

fitzgen commented May 2, 2018

Sketch of what would need to happen:

  • Eagerly fetch / read the wasm in SourceMapConsumer.initialize and save it in some global within lib/source-map-consumer.js

  • Return a promise from SourceMapConsumer.initialize that resolves after the wasm is instantiated, and therefore we can synchronously create new consumers

  • Add a SourceMapConsumer.synchronouseNew (open to bike shedding on the name) function that throws if the wasm has not been initialized, and otherwise creates a new consumer synchronously

@ahmedcharles
Copy link
Author

What if SourceMapConsumer.initialize or SourceMapConsumer.with or any other function that is async accepts the already loaded wasm file? In that scenario, you could just not return a promise and it would just work, no globals required.

@fitzgen
Copy link
Contributor

fitzgen commented May 7, 2018

I would prefer not to expose the wasm directly, since it is an implementation detail, and its interface may change. If we could wrap it up and hide the internals, then that could work.

@jasonLaster
Copy link

@loganfsmyth and I discussed this topic a couple weeks ago because a sync API is important for tools like babel.

@loganfsmyth
Copy link
Contributor

I think the upfront-delay approach would be potentially reasonable in my mind, but it is painful in some cases. I've wanted to explore doing that for babel-register anyway. We'd definitely need a new major version of node-source-map-support that returned a promise that resolved once the WASM was finished loading, then other tooling could do a similar thing.

The main issue for babel-register is that a lot of people use it via node -r babel-register app.js, and I don't believe Node exposes a way for -r items to delay execution, so it would probably require some hackery to make that work by overwriting Node-private stuff :(

@ai
Copy link

ai commented Jun 19, 2018

PostCSS needs sync source map support as well. We use source map in throwing errors.

@devsnek
Copy link

devsnek commented Dec 30, 2018

I think the best option would be to load the wasm into the library source directly instead of distributing it separately. This would be a fairly trivial build step and fix this entire issue. You would need to use the sync interface of WASM (new WebAssembly.Module(buffer)) but it would definitely be worth it.

POC sync SourceMapConsumer https://github.com/devsnek/node-source-map-support/blob/master/source_map.js

@loganfsmyth
Copy link
Contributor

@devsnek The difficulty there is that is that the sync interface isn't guaranteed to work. Chrome for instance throws on anything beyond 4k: webpack/webpack#6475 (comment) While your approach will work on Node, it does not offer a solution for general usage. You're not wrong though, this would likely be the way to got on the Node side of things. On the browser side, I'm curious about compiling the WASM to ASMjs-style code, but I haven't had time to explore it.

@jdalton
Copy link

jdalton commented Dec 31, 2018

The @webassemblyjs packages could be of interest.
It looks like there might be synchronous APIs. \cc @xtuc

@devsnek
Copy link

devsnek commented Dec 31, 2018

@jdalton that's pretty cool. however after a certain point it might just not be worth using wasm anymore :(

@jasonLaster
Copy link

@jdalton can you elaborate?

@loganfsmyth what do you think the current size is?

CC @fitzgen I'm curious what you think?

@loganfsmyth
Copy link
Contributor

@jasonLaster mappings.wasm is 48k at the moment.

I'm also not 100% clear on what @jdalton is suggesting.

@jdalton
Copy link

jdalton commented Jan 1, 2019

I was pointing to various WASM related packages written in JavaScript, like this, which maybe be bundleable and used to side-step limitations imposed on native implementations.

@octogonz
Copy link

octogonz commented Mar 3, 2019

Fetching and compiling WebAssembly is inherently async.

Is there a different implementation that doesn't rely on WASM? I don't really care about extreme performance optimizations. I just want a simple straightforward library that can read a .map file and translate a coordinate, without nasty promises spoiling my beautiful synchronous garden heheh.

@octogonz
Copy link

octogonz commented Mar 3, 2019

Version 0.6.1 has a conventional API and seems pretty usable. Are there any important features missing from that release?

Maybe someone can fork 0.6.1 and maintain a separate version of this package without the WASM dependency.

@SimenB
Copy link
Contributor

SimenB commented Mar 3, 2019

Why can't you keep using 0.6.1?

@octogonz
Copy link

octogonz commented Mar 3, 2019

0.6.1 was published over a year ago. Is it actively maintained?

@octogonz
Copy link

octogonz commented Jun 7, 2020

I noticed a couple other problems with the WebAssembly approach:

  • When debugging code that uses the newer source-map, the calls like this._wasm.exports.original_location_for() are a black box. You cannot step into them using the debugger, or inspect the their implementation easily. Even if we could get that working, I'm not familiar with Rust, and it seems like an odd requirement when the rest of the entire stack is JavaScript.

  • From a security standpoint, the mappings.wasm file contains 48kb of opaque binary data. There is no easy way to decompile it or scan for suspicious code. We simply have to trust that it is the output of building the fitzgen/source-map-mappings project, and that nothing was tampered with along the way.

It totally makes sense to include a WebAssembly binary for people who need these speed optimizations. But it's difficult to understand why the JavaScript reference implementation was completely eliminated.

@fitzgen Would the maintainers consider bringing back the JavaScript implementation and maintaining it alongside the Rust version? If not, maybe it's time to fork this project. This issue has been open for over a year now without resolution. Thanks!

@loganfsmyth
Copy link
Contributor

Even if we could get that working, I'm not familiar with Rust, and it seems like an odd requirement when the rest of the entire stack is JavaScript.

I can appreciate that it's an unexpected speed bump, but I do question how often users would realistically do that. WASM debugging in devtools is something that will improve over time I'm sure.

There is no easy way to decompile it or scan for suspicious code. We simply have to trust that it is the output of building ...

Since WASM runs in a sandbox, it doesn't really have access to anything to do anything anyway AFAIK. A worst all it could do is return the incorrect result.

Would the maintainers consider bringing back the JavaScript implementation and maintaining it alongside the Rust version?

I think maintaining parallel implementations would be a recipe for inconsistent behavior across the module. I have thought about setting up a build step that would compile the WASM into JS code in order to address the sync case.

@octogonz
Copy link

octogonz commented Jun 7, 2020

I think maintaining parallel implementations would be a recipe for inconsistent behavior across the module.

Is source-map an implementation of a specification? If so, there is inherent value in providing a reference implementation written in the language that everybody knows. [Here "everybody" means everybody who installs NPM packages.]

Or is the specification merely "whatever the source-map program code does"? In that case, providing two versions of the code would have some downsides.

@andersk
Copy link

andersk commented Oct 15, 2020

Why can't you keep using 0.6.1?

#370 is a good reason.

@cspotcode
Copy link

In my opinion, the current API, where constructors return a Promise, is a code-smell. It is idiomatic for a constructor to return an instance of the class, such that a = new Foo(); a instanceof Foo.

A better API is for the SourceMapConsumer constructor to be synchronous, and for it to throw an error if the user has not first initialized the wasm module in environments that require async initialization.

Usage in the browser and node can be async like this:

async function createSourceMapConsumer(...) {
    await SourceMapConsumer.initializeModule(); // does not need to be a static method of the class; can be any static (non-instance) function
    return new SourceMapConsumer(...);
}

Usage in node can optionally be synchronous:

function createSourceMapConsumer(...) {
    return new SourceMapConsumer(...);
}

Constructor semantics will be simpler because they will no longer return promises. async function createSourceMapConsumer(), however, can return a promise. I included createSourceMapConsumer for demonstration purposes but it doesn't not necessarily need to be included in source-map's API.

Additionally, in ECMAScript module environments -- browser, node, or otherwise -- the async initialization step can be performed via top-level await in the ESM entrypoint file. The entire module will import async, and the API will be synchronous after that.

@cspotcode
Copy link

@cspotcode/source-map and @cspotcode/source-map-consumer implement a sync API for node using the WASM/rust component while preserving an async API for when it is necessary. For anyone who finds this issue in the future, it may work for you.

@PaleHazy
Copy link

PaleHazy commented Aug 31, 2022

We could add a sync API to create a SourceMapConsumer given that you've already fetched and compiled the wasm. Would that work? If so, I can help you craft a PR that implements this.

Hello, was there anything done regarding this solution specifically for browsers ? I like the idea of pre-fetching the wasm and maps + caching, then carrying on from there synchronously.

Edit:
I looked into this and I achieved this by caching the consumer like so:

let basicConsumer;
//....
     window.sourceMap.SourceMapConsumer.fromSourceMap(cachedSourceMap).then(
        (consumer: any) => {
          basicConsumer = consumer;
        },
      );

//later on 
       basicConsumer.originalPositionFor({
        line: parsed.lineNumber,
        column: parsed.columnNumber,
      });
//....

not sure if this was obvious but works for me.

@lancejpollard
Copy link

lancejpollard commented Jun 5, 2023

@cspotcode can you elaborate? I only see async APIs in those two links.

@lancejpollard
Copy link

lancejpollard commented Jun 5, 2023

Actually it just works, though the docs all say async stuff.

import smc from '@cspotcode/source-map'

const json = JSON.parse(mapContent) as smc.RawSourceMap
const sm = new smc.SourceMapConsumer(json)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.