Skip to content

feat(runtime-sdk): revise type conversion to support bindings more natively#112

Open
ryanking13 wants to merge 7 commits into
mainfrom
gyeongjae/storage-bindings
Open

feat(runtime-sdk): revise type conversion to support bindings more natively#112
ryanking13 wants to merge 7 commits into
mainfrom
gyeongjae/storage-bindings

Conversation

@ryanking13
Copy link
Copy Markdown
Contributor

Note: this includes some breaking changes so it needs a careful look. Also this probaby needs to be merged after #108.

I was experimenting with our current type conversion logic in our SDK to see if a small change is sufficient to make the bindings work more natively. With these changes, I was able to make all the general use cases of storage bindings (R2, KV, D1) work without exposing any Pyodide/JS-specific APIs.

Changes included

  1. Python dict is now converted to JS object by default when passed through the RPC boundary.

This allows python dict to be passed to the RPC function without using to_js. For example,

# before
kv.put(key, value, to_js({"metadata": "blah-blah"})
# after
kv.put(key, value, {"metadata": "blah-blah"})

This change was actually landed in Pyodide 0.29.0, but was not applied in Python workers.

  1. JavaScript Null is now converted to Python None when coming from RPC boundary.

So that users do not need to care about jsnull, None difference.

# before
kv.get("nonexisting") == pyodide.ffi.jsnull
# after
kv.get(nonexisting") == None
  1. JavaScript Object coming from RPC boundary is now converted to Python-dict like object, but their attributes are accessible through ["key"] but also by .key (optional)

(This change is not necessary, but I think it improves the user experience)

This allows mixing .key-style access and ["key"]-style access. This helps accessing attributes when the RPC returns complicated, nested object with JsProxy and python dict mixed.

For example, let's say there is a function that returns the following JS object.

{
  key: string,
  data: Blob,
}

When it passes through the RPC bounday, it will be converted to a Python dict that is

{
  "key": str,
  "data": JsProxy,
}

where Blob is a JsProxy or a Python class that wrap the JsProxy.

In this case, we can now access the Blob attribute with

obj = rpc_call()
obj.data.size

while previously, users needed to

obj["data"].size
  1. Wrapper functions that needs extra type conversion (None => jsnull)

D1.bind function accepts null as a parameter. Passing undefined instead of null is a runtime error. To handle this conversion automatically, I made a wrapper function for .bind to handle this conversion in the background. This means when there are other bindings that needs to handle the null value, we need to create a wrapper function likewise.

Alternatively, we can export jsnull type to the users e.g. from workers import jsnull and let them pass it to the function, but it makes the user experience less ergonomic.


Changes not included

  1. JS function names / parameter names are used as-is.

All the JS function names / parameter names are used as-is, as a camelCase. We do not convert it to Pythonic names.

…flare bindings more natively

- Python dict is now converted to js object by default when passed through the RPC boundary.
- JavaScript Null object is now converted to Python None when coming from RPC boundary.
- JavaScript Object coming from RPC boundary is now converted to Python-dict like object
  but their attributes are accessible through ["key"] but also by .key
- Better support for R2, KV, and D1 bindings support
@ryanking13 ryanking13 requested review from dom96 and hoodmane May 26, 2026 09:56
Copy link
Copy Markdown

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm Bonk, and I've done a quick review of your PR.

This PR adds native SDK support for KV, R2, and D1 bindings by revising type conversion (dict→Object, jsnull→None) and introducing _RPCWrapper / JsDict for ergonomic access patterns.

Issues (by severity)

  1. _RPCWrapper.__getattr__ caches wrappers for non-callable properties, causing self.__class__ to propagate the wrong subclass. When _D1DatabaseWrapper._convert_result receives a JsProxy (e.g., a D1PreparedStatement from .prepare()), it wraps it as _D1DatabaseWrapper(result) because self.__class__ is _D1DatabaseWrapper. The prepared statement then carries the custom bind override. This happens to work for D1, but if any other non-D1 JsProxy is returned from a D1Database method in the future, it would incorrectly get the bind override. Consider using _RPCWrapper explicitly for results that aren't D1Database instances, or check the JS constructor name before choosing the wrapper class.

  2. JsDict has asymmetric read/write behavior for attribute access. d.foo reads from the dict (via __getattr__), but d.foo = val writes to the instance __dict__ (not the dict) since there's no __setattr__ override. This means d.foo = 'bar' followed by d['foo'] will return the old value. Since JsDict is used for read-only result objects this is likely fine in practice, but it could surprise users who try to mutate via attribute syntax. Consider adding a __setattr__ that also writes to the dict, or documenting this as read-only.

  3. Missing newline at end of file in d1_test.py and r2_test.py.

Comment thread packages/runtime-sdk/src/workers/_workers.py
Comment thread packages/runtime-sdk/src/workers/_workers.py
Comment thread packages/cli/tests/bindings-test/src/d1_test.py
Comment thread packages/cli/tests/bindings-test/src/r2_test.py
@ask-bonk
Copy link
Copy Markdown

ask-bonk Bot commented May 26, 2026

I'm Bonk, and I've done a quick review of your PR.

This PR adds native SDK support for KV, R2, and D1 bindings by revising type conversion (dict→Object, jsnull→None) and introducing _RPCWrapper / JsDict for ergonomic access patterns.

I posted a review on PR #112 with 4 inline comments covering 3 issues ranked by severity:

  1. self.__class__ in _convert_result propagates subclass wrappers to all child JsProxy objects_D1DatabaseWrapper wraps prepared statements as _D1DatabaseWrapper too, which happens to work but is fragile. Suggested using _RPCWrapper for non-database results or checking the JS constructor name.

  2. JsDict asymmetric read/writed.foo reads from the dict but d.foo = val writes to the instance __dict__, not the dict. Fine for read-only result objects but worth documenting.

  3. Missing newline at EOF in d1_test.py and r2_test.py.

The test coverage is thorough — good coverage of KV (20 tests), R2 (20 tests), and D1 (18 tests) including edge cases like pagination, multipart uploads, and null binding.

github run

@dom96
Copy link
Copy Markdown
Contributor

dom96 commented May 28, 2026

  1. Python dict is now converted to JS object by default when passed through the RPC boundary.
# before
kv.put(key, value, to_js({"metadata": "blah-blah"})
# after
kv.put(key, value, {"metadata": "blah-blah"})

This is great, though the ideal would be to make the following work:

kv.put(key, value, metadata="blah-blah")

I think this would be the most Pythonic option. Though I understand why it might be challenging. I think it's worth discussing.

  1. JavaScript Null is now converted to Python None when coming from RPC boundary.
  2. JavaScript Object coming from RPC boundary is now converted to Python-dict like object, but their attributes are accessible through ["key"] but also by .key (optional)

This is great, and I think it makes perfect sense. Is this something we can upstream to Pyodide as well?

  1. Wrapper functions that needs extra type conversion (None => jsnull)

What is surprising to me is actually that Pyodide converts None to undefined. Why is that the default? Shouldn't the undefined be the special case?

Is there a path for us to change this? Perhaps with some feature flag in Pyodide so that existing Pyodide users aren't broken by this?

@ryanking13
Copy link
Copy Markdown
Contributor Author

kv.put(key, value, metadata="blah-blah")

Acutally, this already works. Pyodide converts keyword parameters to JS object as a last parameter, which means the following two are equivalent.

kv.put(key, value, { "metadata": "blah-blah" })
kv.put(key, value, metadata="blah-blah")

I added some new tests to make sure if works (See: e202e8d)

I am not sure which one we should document though. Maybe we can discuss this to make sure we have all the examples in our documentation to have the same format

JavaScript Null is now converted to Python None when coming from RPC boundary.
JavaScript Object coming from RPC boundary is now converted to Python-dict like object, but their attributes are accessible through ["key"] but also by .key (optional)
This is great, and I think it makes perfect sense. Is this something we can upstream to Pyodide as well?

Yeah, we already have something similar:

  • For null to None, it hasn't been long since Pyodide introduced jsnull type, and we still have an opt-in flag that converts JS Null to Python None.

  • For Python dict working like a JS object, we have .as_py_json method in JSProxy that makes the key in the JSProxy accessible through ["key"]

However, the problem is that the existing Python workers that run Pyodide 0.28.2 would still have jsnull so we need to keep this conversion logic unless we drop supporting old Python workers in our sdk.

What is surprising to me is actually that Pyodide converts None to undefined. Why is that the default? Shouldn't the undefined be the special case?

Well, I think it makes sense to convert None to undefined.

@dom96
Copy link
Copy Markdown
Contributor

dom96 commented May 29, 2026

Well, I think it makes sense to convert None to undefined.

Why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants