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

Update jsonld.js #279

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

piggyman007
Copy link

I am working on node for ios app (https://code.janeasystems.com/nodejs-mobile) which run on node-chakracore instead of v8.

I found that the spread operator of _setDefaults does not work on node-chakracore, so I would like to change to code by using the basic style (does not affect functional).

Please help to review and approve

I am working on node for ios app (https://code.janeasystems.com/nodejs-mobile) which run on node-chakracore instead of v8.

I found that the spread operator of _setDefaults does not work on node-chakracore, so I would like to change to code by using the basic style (does not affect functional).

Please help to review and approve
@davidlehn
Copy link
Member

I'm not sure what to do here. We like to be free to use fancy new features. So far we've basically felt free to use anything in an "Active LTS" Node.js release that has babel support. Object spread support has been in v8 based Node.js since around 8.3.0. It's also a stage 4 proposal so should be a standard at some point. It's entirely possible we'll want to increase object spread usage which would be a problem for ChakraCore.

It appears ChakraCore is the only JS runtime without object spread support? Seems the future of ChakraCore is uncertain too. How long would we have to keep such a patch around before updating to the modern syntax?

Are other options like using a babel pre-processor too painful in your case?

@piggyman007
Copy link
Author

piggyman007 commented Dec 18, 2018

@davidlehn , my code version support both v8 and chakracore.

Is it good idea to merge it to the main branch?

It should be ok.

@davidlehn davidlehn mentioned this pull request Dec 19, 2018
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.

None yet

2 participants