Skip to content

feat: allow thunked objects to be spreadable #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 24, 2020

Conversation

monotykamary
Copy link
Contributor

Arrays returned in thunks can concatenated with the spread operator while objects cannot. This adds traps for ownKeys and getOwnPropertyDescriptor on ThunkProxy to allow non-iterable objects to be spreadable.

Arrays returned in thunks can concatenated with the spread operator while objects cannot. This adds traps for ownKeys and getOwnPropertyDescriptor on ThunkProxy to allow non-iterable objects to be spreadable.
@ivenmarquardt
Copy link
Contributor

ivenmarquardt commented May 22, 2020

Thanks for contributing! Spreadable thunked objects are useful, of course. However, before I merge this branch I have to take a closer look at your getOwnPropertyDescriptor trap, because it seems to convert every property to be an enumerable/configurable one, even if the wrapped object has non-enumerable ones. Maybe I am just confusing things, so please bear with me..

Preserve property descriptors in objects while still allowing them to be spreadable.
@monotykamary
Copy link
Contributor Author

monotykamary commented May 23, 2020

because it seems to convert every property to be an enumerable/configurable one, even if the wrapped object has non-enumerable ones

Oh, you're right. In that case, we can reflect the getOwnPropertyDescriptor to preserve descriptors.

I've pushed a new commit to 'reflect' this 😅. It will throw a TypeError if the proxy object is not configurable, which I haven't thought up a clean solution for 🤔.

Copy link
Contributor

@ivenmarquardt ivenmarquardt left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@ivenmarquardt ivenmarquardt merged commit 1ae078b into robotroutine:master May 24, 2020
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