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

Fix electron stability by switching from nan to node-addon-api #177

Merged
merged 4 commits into from
Nov 20, 2020

Conversation

McShelby
Copy link
Contributor

Fixes #176

so we are raising our export object from scratch
instead of redefining the nodejieba exports
@yanyiwu
Copy link
Owner

yanyiwu commented Nov 19, 2020

If it is merged, it may cause the version below 7.0.0 to fail. Are there many people using nodejs versions below 7.0?

@McShelby
Copy link
Contributor Author

Hard to tell.

I see your module used as dependency for a lot of other chinese related node modules. That makes the impact hard to predict.

Node 6 was an LTS release and is unsupported since 2019-04-30.
Node 7 is from late 2016.
Current Node LTS is 14.x
Electron 10 is using 12.x
Electron 8 (as used in VSCode) is also using 12.x

For me, this seems worth the risk.

@yanyiwu yanyiwu merged commit dbd0e23 into yanyiwu:master Nov 20, 2020
@yanyiwu
Copy link
Owner

yanyiwu commented Nov 20, 2020

Thank you very much. If you are also a cryptocurrency enthusiast, for example, if you have an EOS account, I hope to transfer a little EOS tokens through the blockchain to express my gratitude forever.

@yanyiwu
Copy link
Owner

yanyiwu commented Nov 21, 2020

https://travis-ci.org/github/yanyiwu/nodejieba/builds/744923635

@McShelby after i taged v2.5.0 , it failed. it is weird because the code is unchanged. can you help to find out what's the problem ?

@McShelby
Copy link
Contributor Author

Seariously puzzling. It may have something to do with the --fallback-to-build switch. But at the moment I am only guessing.

I was already wondering, that it worked with Node 7.1.0. I did not expect this. Now with this error, I have the fealing, that the travis job of my bugfix branch was using the precompiled version but did not run the compile at all.

@McShelby McShelby mentioned this pull request Nov 21, 2020
McShelby added a commit to McShelby/nodejieba that referenced this pull request Nov 21, 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.

Fix electron stability
2 participants