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

Downgrade to yn v3 to avoid v4's node 10 requirement #942

Merged
merged 3 commits into from
Jan 13, 2020
Merged

Conversation

cspotcode
Copy link
Collaborator

Fixes #940

@coveralls
Copy link

coveralls commented Jan 13, 2020

Coverage Status

Coverage increased (+0.1%) to 78.864% when pulling 2a2d4d2 on fix-940 into b90754e on master.

src/index.ts Outdated
*/
function yn (value: string | undefined) {
const parsed = ynModule(value)
return parsed === null ? undefined : parsed
Copy link
Member

@blakeembrey blakeembrey Jan 13, 2020

Choose a reason for hiding this comment

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

We could also just use nullish coalescing within the DEFAULTS object nowadays.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah good point. Even using ??, it's a bit more DRY to keep it within this helper function, and it matches the API of yn v4. Do you still want to get rid of the helper function and move it all to the DEFAULTS object literal?

@blakeembrey blakeembrey merged commit 8db216c into master Jan 13, 2020
@blakeembrey blakeembrey deleted the fix-940 branch January 13, 2020 18:05
@Borewit
Copy link

Borewit commented Jan 13, 2020

Thank you so much @cspotcode & @blakeembrey!

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.

Node engine V8 compatibility is broken
4 participants