-
Notifications
You must be signed in to change notification settings - Fork 97
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
SDK-49 Add custom section for sourceMappingURL #124
Conversation
Hm, since this value should only be needed at the outermost part of module generation it doesn't seem necessary to store it in the environment. It's a bit annoying that this param pollutes all the compile & pipeline API now. Maybe it's time to aggregate some of the compile parameters (mode, name, url) into a config record? |
We create modules for local actors, so that part might be needed. But is it even correct to use the same source map for the surrounding and the local modules? Presumably not, or at least not implemented like now. (The sourcemap spec seems to include a So maybe Or we could ditch embedding URLs altogether, and Of course, if we ditch local actors soon anyways, then this is all moot. Another, orthogonal thought: Is compile time really the right time to specify the URL of the source map? Isn’t it more likely deploy time, i.e. when you upload a working program “somewhere” that you know where to publish the source map? If so, then maybe the URL should not be embedded by the compiler, but rather a separate tool that acts on the |
Agreed that compile time may be the wrong point. |
I think the concerns above are all valid. However, I still think the best developer experience would be to specify that a source map should be produced, and then by default make the resulting files ready to be consumed by a browser. I think this means:
Does anyone feel strongly against doing the above? If not, are there ways I can improve on what's here? |
I'll look at addressing both of these. |
As far as I know there's a In my experience it's common for a single file to containing many modules, classes, etc to be loaded by a client/browser with it's corresponding source map containing data on multiple source files. Is that what you're getting at, or was it something else? |
I remember why I did this; it seemed cleaner than polluting |
But Here is the line where we throw away the source map from the embedded module at the moment: https://github.com/dfinity-lab/actorscript/blob/822d85a3bec1652119a4bb807022abe06616a823/src/compile.ml#L3860 There are voices for an alternative plans where embedded modules are written to separate files, and somehow bundled in by way of a manifest, and uploaded to the chain along side of each other. If that happens, then it is more obvious where to write the embedded source maps to.
Ah, so it wouldn’t be a full URL, just the basename? |
It would be just the filename, e.g. WebAssembly/design#1051 says under Linking generated code to source maps:
So I think for now, just the filename is a reasonable default |
Could you help me understand what these local/embedded modules are and how they are special (regarding source maps)? I'm not sure I agree with "clearly have their own URL"; I think if these modules exist in
I'm trying to work with what browsers have implemented today, which appears to be what is described in WebAssembly/design#1051. |
Let me use a Javascript analogy. If you have
If you run this code, the browser loads the embedded JavaScript file as a separate file, but it is no longer connected to |
I see. Thanks for the explanation. I think if the base64-encoded JS includes a sourceMappingURL that tells the browser that the source map is in |
(Provided that the source map for the base64-encoded JS in in |
Also, I fail to see how it is useful to just write Here is an alternative proposals:
This way, the SDK just needs to collect all these In fact, we should maybe even include the source there (Source maps can optionally include the original source.) This way, there is never confusion about picking the wrong source file or source map for a given on-chain actor. |
But a source map is always only for one (produced) file. What should the entry “line 1 position 1” in that map refer to – the outer JS or the inner JS? |
I've definitely been thinking about this too much in terms of browsers and not our network. Thanks for pointing this out. |
I thought a single source map file could represent multiple source files but perhaps that's not the case. |
As far as I know, the only way to tell a browser where to find a source map is through the mechanisms I shared above, described in WebAssembly/design#1051 under Linking generated code to source maps:
Which brings up the question of how these files are going to be served when the debugger is attached to V8, and what control we have over that (I'm assuming little) I'm still making progress on doing that and have been documenting my findings along the way here: https://dfinity.atlassian.net/browse/SDK-22 |
I think I should switch focus back to working on that and see what it reveals. Is there any progress to be made here in the meantime based on the alternative proposals? |
multiple sources yes (in our case: ActorScript), but not multiple outputs (in our case: Wasm modules).
I think we should pause this one until we actually have the debugger attached to V8 in the node. I suspect that by the time that runs we have a better understanding where the debugger would look for source maps in that case. Is there HTTP involved? Maybe there is a place to inject Or maybe we can create a small chrome plugin that handles a custom URL scheme ( |
That's what I mean by "I think I should switch focus back to working on that and see what it reveals." Sorry it was unclear. Glad we are on the same page though :) |
We now have a test app attachable by Chrome on the |
Closing this, it’s been months since anybody talked about source maps… |
## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@3cc51be5...9a7bcb2f](dfinity/ic-hs@3cc51be...9a7bcb2) * [`9a7bcb2f`](dfinity/ic-hs@9a7bcb2) remember last trap also if on_cleanup succeeds ([dfinity/ic-hs#124](https://togithub.com/dfinity/ic-hs/issues/124))
## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@3cc51be5...8384593e](dfinity/ic-hs@3cc51be...8384593) * [`9a7bcb2f`](dfinity/ic-hs@9a7bcb2) remember last trap also if on_cleanup succeeds ([dfinity/ic-hs#124](https://togithub.com/dfinity/ic-hs/issues/124)) * [`8384593e`](dfinity/ic-hs@8384593) scale cycle cost of http_request calls based on subnet size ([dfinity/ic-hs#123](https://togithub.com/dfinity/ic-hs/issues/123))
## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@3cc51be5...8384593e](dfinity/ic-hs@3cc51be...8384593) * [`9a7bcb2f`](dfinity/ic-hs@9a7bcb2) remember last trap also if on_cleanup succeeds ([dfinity/ic-hs#124](https://togithub.com/dfinity/ic-hs/issues/124)) * [`8384593e`](dfinity/ic-hs@8384593) scale cycle cost of http_request calls based on subnet size ([dfinity/ic-hs#123](https://togithub.com/dfinity/ic-hs/issues/123))
This is based on WebAssembly/design#1051.
I doubt using the
env
the way I have is the best/correct way to do this, so would appreciate some feedback on that.We may wish to add support for providing a base URL like emscripten does.
I'm running into what seems to be an unrelated issue (#123) since a version of this was working at an older commit.