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

Fixes #112 deprecated V8 API usage (also updates documentation from using var to const) #113

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

riyadshauk
Copy link

For reference on currently acceptable V8 API usage:

bcoin-org/bcrypto#7

nodejs/node@7ce749d722

and of course: https://docs.google.com/document/d/1g8JFi8T_oAE_7uAri7Njtig7fKaPDfotU6huOa1alds/edit

The above resources were used to fix the breaking, deprecated usage of the V8 API in time.cc.

Please pull my changes into master : )

@luavixen
Copy link

You are a god. Thank you so much for fixing this and making my life a lot easier!

@riyadshauk
Copy link
Author

Yeah, no problem.

Like I said in the linked issue, just for the time being, until this repo is fixed, you can use this in your package.json:

  "dependencies": {
    "node-time": "github:riyadshauk/node-time"
  }

@theaccordance
Copy link

@riyadshauk your fix addressed the blocker I encountered while upgrading the version of Node from 8.12 to 12.16 (LTS) on a old monolith project, thank you for taking the time to solve the problem!

@TooTallNate I know OSS maintenance is a thankless job, but if I ask nicely, can we get a little bit of attention over here to review & accept this PR? Pointing to @riyadshauk's Github Repo in the package.json manifest is a security vulnerability that I'd rather not introduce into my production app & I'd like to avoid forking the solution to republish a completely new npm package

giphy

@ZeroJsus
Copy link

Yeah, no problem.

Like I said in the linked issue, just for the time being, until this repo is fixed, you can use this in your package.json:

  "dependencies": {
    "node-time": "github:riyadshauk/node-time"
  }

Thank you!

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.

4 participants