run wavedash dev with local sdk - #56
Conversation
The dev shell hardcodes cdn.jsdelivr.net/npm/@wvdsh/sdk-js@<pinned version>, so trying an SDK change under `wavedash dev` meant publishing it first. This points the injected script tag at a local bundle served from the dev server's own origin, which COEP accepts without the CORP header a cross-origin build would need. Takes the bundle or any directory holding it, so an sdk-js checkout and its dist/ both work, and resolves at startup so a wrong path fails immediately rather than as a broken script tag once the game is already loading. The file is read per request, so rebuilding sdk-js only needs a page reload. With no flag the pinned CDN url is built exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The repo is public, so a test named for a hidden flag documents the flag it is hiding. What it uniquely covered — which commands carry --upload-source, and that godot-plugin is its only value — is not worth advertising that in the test names. `upload_source_parses_the_plugin_and_defaults_to_the_cli` still covers parsing the value and defaulting to the CLI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @The64thRealm's task in 4m 17s —— View job Code review:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| } | ||
|
|
||
| /// Same-origin, which COEP allows without the CORP a cross-origin build needs. | ||
| const LOCAL_SDK_URL: &str = "/__wavedash/sdk.js"; |
There was a problem hiding this comment.
Serving the bundle under a renamed URL breaks its sourcemap, which is exactly what you want when debugging a local sdk-js build. A dev dist/inject.global.js ends with //# sourceMappingURL=inject.global.js.map; the browser resolves that relative to /__wavedash/sdk.js, requests /__wavedash/inject.global.js.map, which falls through to handle_static and 404s — so SDK frames stay minified/unmapped in devtools.
Cheapest fix: serve any sibling *.map next to cfg.sdk_js from /__wavedash/<name> (or route the bundle at /__wavedash/inject.global.js and serve the map alongside it).
| }; | ||
| match std::fs::read(path) { | ||
| Ok(bytes) => respond(StatusCode::OK, "text/javascript; charset=utf-8", None, bytes), | ||
| Err(e) => respond( |
There was a problem hiding this comment.
The read-failure path fails silently from the user's point of view. The doc comment invites the workflow "rebuild sdk-js, reload the page", so reloading mid-rebuild (file momentarily missing, or read back truncated) is a realistic race. In either case window.Wavedash never gets defined, and dev.js's initGate starts with if (window.Wavedash.gameLoaded) — it throws TypeError inside the DOMContentLoaded listener, so the boot overlay never mounts and no message is shown. The developer gets a black page and has to find the 500 in the network tab.
Consider responding with JS that surfaces the reason through the existing gate, e.g. body window.__wavedashBootError = "could not read <path>: <err>"; with Content-Type: text/javascript (status 200 or 500 — the script body is what the gate can act on).
| ); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
This PR deletes upload_source_is_hidden_and_only_offers_the_godot_plugin, which is unrelated to --sdk-js and leaves a coverage hole: nothing now asserts that --upload-source is hide-set, nor that wavedash dev's copy restricts values to godot-plugin (the surviving upload_source_parses_the_plugin_and_defaults_to_the_cli only exercises build push). Concretely, dropping hide = true on either arg, or adding a new UploadSource variant, now passes CI silently.
If the concern is the test name advertising a hidden flag, renaming it (e.g. every_build_creating_command_agrees_on_its_source_arg) keeps the guard without the signal.
|
Claude finished @The64thRealm's task in 4m 18s —— View job Code review:
|
|
|
||
| const DEFAULT_CONFIG: &str = "./wavedash.toml"; | ||
|
|
||
| fn resolve_sdk_js(path: PathBuf) -> Result<PathBuf> { |
There was a problem hiding this comment.
resolve_sdk_js is the one piece of new logic here with real branching — dir → dist/inject.global.js → <dir>/inject.global.js → direct-file passthrough → error — and it's the piece a user hits first and most often (--sdk-js ~/code/sdk-js vs --sdk-js ~/code/sdk-js/dist vs a direct path). It has no test, while the much thinner inject_url switch got one (a_local_bundle_is_served_from_this_origin).
tempfile is already a dev-dependency and src/config.rs:1026 has the tempfile::tempdir() pattern to copy, so this is cheap:
#[test]
fn a_local_sdk_js_resolves_from_a_repo_root_a_dist_dir_or_a_direct_file() {
let dir = tempfile::tempdir().expect("temp dir");
let root = dir.path();
let dist = root.join("dist");
std::fs::create_dir(&dist).unwrap();
let bundle = dist.join("inject.global.js");
std::fs::write(&bundle, "//").unwrap();
// The repo root a developer actually types.
assert_eq!(resolve_sdk_js(root.to_path_buf()).unwrap(), bundle);
// The dist dir — must not become dist/dist/inject.global.js.
assert_eq!(resolve_sdk_js(dist.clone()).unwrap(), bundle);
// A direct path is taken as-is.
assert_eq!(resolve_sdk_js(bundle.clone()).unwrap(), bundle);
// Nothing to serve is an error, not a 500 at request time.
assert!(resolve_sdk_js(root.join("nope")).is_err());
}The dist case is the one worth locking down: the candidate order is what keeps it from resolving dist/dist/inject.global.js, and reordering the vec! would silently break it.
| /// Same-origin, which COEP allows without the CORP a cross-origin build needs. | ||
| const LOCAL_SDK_URL: &str = "/__wavedash/sdk.js"; | ||
|
|
||
| fn inject_url(sdk_js: Option<&Path>) -> String { |
There was a problem hiding this comment.
inject_url never reads the path — only whether it's Some. That makes Option<&Path> a boolean in a path's clothing, and it's threaded through shell and inject_sdk purely to reach this match, which is what forced three signature changes and four call-site edits in this diff.
Since ServeConfig is built once and the URL is a pure function of sdk_js, resolving it at construction collapses all of that:
// in ServeConfig
/// The SDK bundle URL injected into every page — local when `sdk_js` is set.
pub inject_url: String,Then shell(&cfg.inject_url, ...) / inject_sdk(&cfg.inject_url, ...) take a &str, inject_url(sdk_js) disappears, and the "is there a local build" decision happens once next to resolve_sdk_js instead of on every request. It also removes the mild misdirection for the next reader, who currently has to follow Option<&Path> two frames down to learn the path never lands in the URL.
Not worth blocking on — but if you'd rather keep the current shape, fn inject_url(local: bool) at least says what the parameter is doing.
1 participant
For me to run wavedash dev with my local js-sdk without needing to bump the pin / bump the sdk-js version / do weird shenanigans with my files