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

Unable to install varsion 2.4.1 on Ubuntu #29

Closed
gerasiz opened this issue Oct 22, 2021 · 5 comments
Closed

Unable to install varsion 2.4.1 on Ubuntu #29

gerasiz opened this issue Oct 22, 2021 · 5 comments

Comments

@gerasiz
Copy link

gerasiz commented Oct 22, 2021

Until version 2.4.0 everything was fine. Trying to install version 2.4.1 on Ubuntu produces this error:

In file included from ../src/keymapping.cc:10:
../src/keymapping.cc: In function ‘napi_value__* vscode_keyboard::Init(napi_env, napi_value)’:
../src/keymapping.cc:56:18: error: ‘napi_set_instance_data’ was not declared in this scope; did you mean ‘napi_new_instance’?
56 | NAPI_CALL(env, napi_set_instance_data(env, data, DeleteInstanceData, NULL));
| ^~~~~~~~~~~~~~~~~~~~~~
../src/common.h:64:10: note: in definition of macro ‘NAPI_CALL_BASE’
64 | if ((the_call) != napi_ok) {
| ^~~~~~~~
../src/keymapping.cc:56:3: note: in expansion of macro ‘NAPI_CALL’
56 | NAPI_CALL(env, napi_set_instance_data(env, data, DeleteInstanceData, NULL));
| ^~~~~~~~~

I tried with different NodeJS versions: 10.19.0, 10.20.0 and 12.4.1. I would appreciate any suggestions or fixes since this is a dependency of another package that I use. Thanks in advance!

@alexdima
Copy link
Member

alexdima commented Oct 22, 2021

I suggest using 2.4.0 or 2.3.0 for now. We have a problem (unclear what the problem is) with 2.4.1. We also had to revert adopting it in vscode. See microsoft/vscode#135108

@gerasiz
Copy link
Author

gerasiz commented Oct 22, 2021

The problem is that a dependency that we use (and do not have control) depends on this package using the "^" prefix, so yarn.lock will be updated with the latest minor version. I am not sure if there is a way to control this.

For now this not urgent because it only affects Electron related dependencies that we are not using yet and this is not breaking the browser-only build, so I think we can wait for the fix.

Thanks for the reply!

@alexdima
Copy link
Member

alexdima commented Oct 22, 2021

I tried with different NodeJS versions: 10.19.0, 10.20.0 and 12.4.1

I think this now uses some napi features that only came out with node v14.x (https://github.com/microsoft/node-native-keymap/pull/28/files#diff-d019e3cbfb2cb2915635f46e03bbd0c11add737fba4fc3cd35a3d562f31fe6f2), so the problem we have been running into with vscode integration tests might be unrelated to your problem.

Unfortunately, I did not catch this in time. 2.4.1 should have been 3.0.0 to respect semver. I will look into fixing this by pushing a 2.4.2 that would be the same as 2.4.0.

@gerasiz
Copy link
Author

gerasiz commented Oct 22, 2021

I can confirm that native-keymap installs correctly using nodeJS 14. The problem is that in my case the package that depends on native-keymap is from Theia IDE, they recommend NodeJS 12, and I tried building with NodeJS 14 but it does not work well, so I think it will affect other people.

So I think the right thing to do is as you said, put these changes in version 3. Thanks!

@alexdima
Copy link
Member

Sorry again fro the breakage.

I've published native-keymap@2.5.0 back with the bits from native-keymap@2.3.0 and we'll republish all these new breaking changes under native-keymap@3.x

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

No branches or pull requests

2 participants