Updates Python JS interop docs - adds import_from_javascript#28395
Updates Python JS interop docs - adds import_from_javascript#28395mikenomitch wants to merge 1 commit intoproductionfrom
Conversation
|
This PR requires additional review attention because it affects the following areas: RedirectsThis PR changes current filenames or deletes current files. Make sure you have redirects set up to cover the following paths:
|
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
/bonk thoroughly review this PR. strongly prefer suggestions via the |
There was a problem hiding this comment.
Review Summary
This is a solid PR that renames ffi.mdx → javascript-interop.mdx, updates all internal cross-references, adds a redirect, and introduces documentation for the import_from_javascript API. The changes are well-structured and I confirmed there are no stale references to the old /workers/languages/python/ffi/ path remaining anywhere in the repo.
What looks good
- Redirect is correctly set up:
/workers/languages/python/ffi/→/workers/languages/python/javascript-interop/301 - All internal links to the old FFI page have been updated across 6 files (basics, index, packages, compatibility flag, workflows bindings)
- Heading anchor update in
bindings.mdx(#use-bindings-from-python-workers) correctly matches the new heading text injavascript-interop.mdx - No stale references — I grep'd the entire repo on the PR branch and confirmed zero remaining references to
python/ffi - The
import_from_javascriptAPI is consistent with its existing usage insrc/content/docs/workers/runtime-apis/bindings/index.mdx - Heading levels are correct (H2 → H3 → H2, no skips)
- MDX syntax is clean, no unescaped special characters
Issues found (see inline comments)
- [nit] Missing
descriptionfrontmatter — the page title changed but still has nodescriptionfield (pre-existing, but worth adding since the page is being overhauled) - [suggestion] The first
import_from_javascriptcode example could be tightened —addressis used but never defined - [nit, pre-existing] "see out the" typo on line 107 — since the surrounding content is being touched, this is a good opportunity to fix it
- [nit, pre-existing]
[here]link text on line 20 violates the style guide — same opportunity to fix
Other observations (not blocking)
- The JSONC trailing comma additions in
basics.mdx,javascript-interop.mdx, andbindings.mdxare valid JSONC inside<WranglerConfig>blocks — these are fine - The
create_batchline-wrapping change inbindings.mdxis a pure formatting improvement and renders the same
src/content/docs/workers/languages/python/javascript-interop.mdx
Outdated
Show resolved
Hide resolved
|
Additional nit (pre-existing): Line 107 of
This was present in the original |
|
Review complete. Here is a summary of what I found: Review of PR #28395Overall: This is a clean, well-executed PR. The rename from What I verified
Suggestions submitted (3 inline + 1 comment)
|
66cd113 to
f6e9f32
Compare
Summary
Clarifies JS interop for Python