Simplify "named intercept" #1585
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The fact that they are serialized is implied by the type, `JsValue`, which always contains a serialized value.
`setV8Serialized()` is a capnp data setter. It will always make a copy, it does not take ownership. Also add a comment about avoiding the redundant copy in the future.
This functionality is not used by JS RPC as we have no way to know in advance what methods actually exist at the remote end. Moreover, methods of a class are normally not expected to be enumerable anyway, so even if it could know what methods exist, it'd still be returning an empty list here. Since JS RPC is the only anticipated use case for named intercept at this time, I'd rather remove this unused functionality for now, to simplify further refactoring. We can bring it back in the future if a use case is found.
There's no longer any need to inherit this. JSG_NAMED_INTERCEPT works fine without it, since it always invokes `getNamed()` directly on the particular subclass via templates.
This makes the identifier `getNamed` no longer magic. Instead, the caller specifies whatever name they want.
The type is wrapped just like the return value of any function would be.
The named intercept callback is essentially the same as what `GetterCallback` handles, except with a name passed in. Aside from handling the name, there's no reason this code should be different from `GetterCallback`.
…pt functionality.
I think this is a much more intuitive name.
justin-mp
reviewed
Jan 29, 2024
| // Configures the resource type to implement named property interception. | ||
| // @see the definition of jsg::NamedIntercept in resource.h for more information. | ||
| #define JSG_NAMED_INTERCEPT(method) \ | ||
| // Declares a wildcart property getter. If a property is requested that isn't already present on |
There was a problem hiding this comment.
nit: s/wildcart/wildcard/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the change we discussed previously, to allow the named property getter to return an arbitrary wrappable value.
I also came up with what I think is more intuitive name for this feature: "Wildcard property"