fix(axios): restore typed async response access#6852
Conversation
📝 WalkthroughWalkthroughAxios default exports now retain typed static methods, while async Axios response properties are preserved through native-instance lowering and compilation. Native HEAD and OPTIONS requests are wired through the runtime, with unit and release tests covering response access and isolation from unrelated handles. ChangesAxios integration fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TypeScript
participant AxiosNative
participant ResponseLowering
participant Runtime
TypeScript->>AxiosNative: call axios.get, axios.head, or axios.options
AxiosNative->>Runtime: return response promise
Runtime-->>TypeScript: resolve awaited response
TypeScript->>ResponseLowering: access status or data
ResponseLowering-->>TypeScript: preserve native response dispatch
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
099403c to
9a4c5fc
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/perry-ext-axios/src/lib.rs (1)
14-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRe-export NaN-boxing tags from
perry-ffiinstead of duplicating them here.
TAG_UNDEFINEDis stable ABI documentation inperry-ffi, but the extension crate still hand-represents that ABI bit pattern. Add/forward the needed tag constants fromperry-ffiand import them fromcrates/perry-ext-axios/src/lib.rsso this dispatch code cannot drift from its ABI contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-ext-axios/src/lib.rs` around lines 14 - 65, Remove the locally duplicated TAG_UNDEFINED definition in the axios extension and reuse the corresponding NaN-boxing tag exported by perry-ffi. Add or re-export the required tag constant in perry-ffi, then import that symbol in axios_response_property_dispatch so the statusText null-pointer path uses the shared ABI definition.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/perry-ext-axios/src/lib.rs`:
- Around line 14-65: Remove the locally duplicated TAG_UNDEFINED definition in
the axios extension and reuse the corresponding NaN-boxing tag exported by
perry-ffi. Add or re-export the required tag constant in perry-ffi, then import
that symbol in axios_response_property_dispatch so the statusText null-pointer
path uses the shared ABI definition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 433a2c62-3257-4662-a0f3-48d74b57940c
📒 Files selected for processing (5)
changelog.d/6852-axios-async-response-properties.mdcrates/perry-api-manifest/src/emit.rscrates/perry-ext-axios/src/lib.rsdocs/api/perry.d.tstests/release/packages/axios-get/package.json
9a4c5fc to
d309f8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-hir/src/js_transform/local_natives.rs`:
- Line 1349: Update the axios method match in the native response tagging logic
to include head and options alongside get, post, put, delete, patch, and
request, ensuring their results are classified as Response and receive the
existing response-property lowering.
In `@crates/perry-hir/tests/axios_response_property_lowering.rs`:
- Around line 5-44: Add an equivalent cargo-test-visible unit regression test
alongside the local-native lowering implementation, reusing the awaited Axios
response property scenario and assertions from
awaited_axios_response_properties_keep_native_dispatch. Keep the existing
integration test unchanged while ensuring the unit test exercises
fix_local_native_instances and verifies native Response dispatch for both status
and data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 370c4222-7871-490f-a283-b5be0fe553bc
📒 Files selected for processing (9)
changelog.d/6852-axios-async-response-properties.mdcrates/perry-api-manifest/src/emit.rscrates/perry-hir/src/js_transform/local_natives.rscrates/perry-hir/tests/axios_response_property_lowering.rscrates/perry/src/commands/compile/collect_modules.rsdocs/api/perry.d.tstests/release/packages/axios-get/entry.tstests/release/packages/axios-get/expected.txttests/release/packages/axios-get/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/release/packages/axios-get/package.json
- crates/perry-api-manifest/src/emit.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/release/packages/axios-get/entry.ts (1)
28-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the HTTP verbs, not only their status codes.
Because the fixture returns
200for every request to/json, an implementation that routesaxios.headoraxios.optionsthrough GET would still pass. Recordreq.methodor return method-specific responses and assertGET,HEAD,OPTIONSin the fixture output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/release/packages/axios-get/entry.ts` around lines 28 - 32, Update the `/json` fixture and assertions in the axios request test to verify HTTP methods rather than status codes alone. Capture `req.method` in the fixture output (or provide method-specific responses) and assert that the GET, HEAD, and OPTIONS calls produce `GET`, `HEAD`, and `OPTIONS` respectively, while preserving the existing status checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-ui-android/src/stdlib_stubs.rs`:
- Around line 71-79: Update js_axios_head and js_axios_options to match the
generated I64 -> I64 ABI by accepting the URL handle parameter and routing each
call through Android’s unsupported/rejected-Promise path; alternatively remove
these stubs if those calls are intentionally unreachable on Android. Do not
return raw 0 as a Promise handle.
---
Nitpick comments:
In `@tests/release/packages/axios-get/entry.ts`:
- Around line 28-32: Update the `/json` fixture and assertions in the axios
request test to verify HTTP methods rather than status codes alone. Capture
`req.method` in the fixture output (or provide method-specific responses) and
assert that the GET, HEAD, and OPTIONS calls produce `GET`, `HEAD`, and
`OPTIONS` respectively, while preserving the existing status checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 75361def-c2c1-4b26-b866-9d9a261ffd6b
📒 Files selected for processing (10)
changelog.d/6852-axios-async-response-properties.mdcrates/perry-codegen/src/lower_call/options/fetch.rscrates/perry-codegen/src/runtime_decls/stdlib_ffi/third_party.rscrates/perry-ext-axios/src/lib.rscrates/perry-hir/src/js_transform/local_natives.rscrates/perry-hir/tests/axios_response_property_lowering.rscrates/perry-stdlib/src/axios.rscrates/perry-ui-android/src/stdlib_stubs.rstests/release/packages/axios-get/entry.tstests/release/packages/axios-get/expected.txt
🚧 Files skipped from review as they are similar to previous changes (3)
- changelog.d/6852-axios-async-response-properties.md
- crates/perry-hir/tests/axios_response_property_lowering.rs
- crates/perry-hir/src/js_transform/local_natives.rs
| #[no_mangle] | ||
| pub extern "C" fn js_axios_head() -> i64 { | ||
| 0 | ||
| } | ||
|
|
||
| #[no_mangle] | ||
| pub extern "C" fn js_axios_options() -> i64 { | ||
| 0 | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'js_axios_(head|options)' \
crates/perry-codegen/src/runtime_decls/stdlib_ffi/third_party.rs \
crates/perry-codegen/src/lower_call/options/fetch.rs \
crates/perry-ui-android/src/stdlib_stubs.rsRepository: PerryTS/perry
Length of output: 2771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== android stdlib stubs relevant section =="
fd -a 'stdlib_stubs.rs' . | sed 's#^\./##' | while read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,130p' "$f" | cat -n
done
echo "== android promise/runtime stubs/search for axios handlers =="
rg -n -C 2 'js_axios_(get|head|options)|axios|JsValue|promise|rejection|null|0' crates/perry-ui-android/src || true
echo "== android call/ffi setup search =="
rg -n -C 2 'extern "C"|#[no_mangle]|std::ffi::|CStr|CString|handle|promise|await|JsValue|FfiValue|ReturnType|Return' crates/perry-ui-android/src crates/perry-codegen/src -g '*.rs' | head -n 300Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate stdlib_stubs.rs =="
fd 'stdlib_stubs.rs' . -t f
echo "== android stdlib_stubs.rs lines 1-140 =="
STUBS=$(fd 'stdlib_stubs.rs' crates/perry-ui-android/src -t f | sort | head -n 1)
if [ -n "$STUBS" ]; then
wc -l "$STUBS"
sed -n '1,140p' "$STUBS" | cat -n
fi
echo "== exact axios declarations in perry-codegen =="
sed -n '60,85p' crates/perry-codegen/src/runtime_decls/stdlib_ffi/third_party.rs | cat -n
echo
sed -n '155,180p' crates/perry-codegen/src/lower_call/options/fetch.rs | cat -n
echo "== android promise/extern/ffi stub-related symbols =="
python3 - <<'PY'
from pathlib import Path
root=Path('crates/perry-ui-android/src')
symbols = {'js_axios_get','js_axios_head','js_axios_options','js_axios_create','js_axios_delete','js_make_rejected_promise','js_make_resolved_promise','promise'}
for p in sorted(root.rglob('*.rs')):
txt=p.read_text(errors='replace')
hits=[i for i,l in enumerate(txt.splitlines(),1) if any(s in l for s in symbols) or 'fn ' in l and any(s in l.split('(')[0] for s in symbols)]
if hits:
print(f'--- {p}')
for i in hits:
print(f'{i}: {txt.splitlines()[i-1]}')
PY
echo "== check exported stub functions and call graph =="
rg -n -C 2 'pub extern "C" fn (js_axios_get|js_axios_head|js_axios_options|js_axios_create|js_axios_delete)|fn js_make.*promise|call.*js_axios|axios_handle|axios.*promise|nanbox_pointer' crates/perry-ui-android/src crates -g '*.rs' | head -n 500Repository: PerryTS/perry
Length of output: 49076
Match the Android Axios stubs to the generated call ABI.
js_axios_head and js_axios_options are generated with an I64 -> I64 call passing the URL handle, but these no-op stubs are () -> i64, so the URL is discarded and the returned raw 0 is NaN-boxed as a Promise handle. Add the unused i64 parameter and route through Android’s unsupported/rejected-Promise path, or remove these from the stdlib stubs if the calls are intentionally unreachable on Android.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-ui-android/src/stdlib_stubs.rs` around lines 71 - 79, Update
js_axios_head and js_axios_options to match the generated I64 -> I64 ABI by
accepting the URL handle parameter and routing each call through Android’s
unsupported/rejected-Promise path; alternatively remove these stubs if those
calls are intentionally unreachable on Android. Do not return raw 0 as a Promise
handle.
Summary
Fix Axios editor types and preserve response properties through async lowering without exposing them on unrelated native handles.
Changes
Responsenative-instance metadata before async lowering splits awaited values across synthetic locals.status,statusText, anddatareads to the existing Axios accessors.axios.head()andaxios.options()support across optimized and bundled runtime paths.node:cryptoregression check for handle collisions.Related issue
Fixes #6851
Test plan
cargo build --release -p perrycargo test -p perry-hir --test axios_response_property_loweringcargo test -p perry-api-manifestcargo check -p perry-ext-axiosPERRY_NO_CACHE=1 tests/release/packages/axios-get/fixture.shtsc --noEmitagainstdocs/api/perry.d.tsgit diff --checkOutput
Checklist