Skip to content
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

js-sys: add Object.fromEntries #1456

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@bakkot
Copy link

bakkot commented Apr 14, 2019

Fixes #1453.

I couldn't actually get tests to pass, possibly because Node does not yet support this function.

@bakkot

This comment has been minimized.

Copy link
Author

bakkot commented Apr 14, 2019

Yeah, looks like CI has the same problem as I did locally. I'd appreciate input from a maintainer here - to my (uneducated) eye, it looks like tests in CI are being run in Firefox, but Firefox has had support for Object.fromEntries since version 63 (released in October 2018) and Azure's Ubuntu agent claims claims to be running Firefox 66.

@alexcrichton

This comment has been minimized.

Copy link
Collaborator

alexcrichton commented Apr 15, 2019

Oh so crates/js-sys/tests/wasm/* is actually all executed in Node.js which may explain the discrepancy here, but can you try adding the tests to crates/js-sys/tests/headless.rs as those should be run in Firefox on Azure?

@bakkot

This comment has been minimized.

Copy link
Author

bakkot commented Apr 15, 2019

Hm. I believe Node 12 will include v8 7.4, which includes Object.fromEntries. It's due to be cut on April 23. So maybe it'd be simpler to just wait for that?

@alexcrichton

This comment has been minimized.

Copy link
Collaborator

alexcrichton commented Apr 15, 2019

Seems reasonable to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.