Make code compilable on Node 19 #134
Conversation
| @@ -34,7 +34,7 @@ void InitConversions(Local<Object> exports) { | |||
| v8::Local<v8::Object> bufferView; | |||
| bufferView = node::Buffer::New(Isolate::GetCurrent(), point_transfer_buffer, 0, 2 * sizeof(uint32_t)).ToLocalChecked(); | |||
| auto js_point_transfer_buffer = node::Buffer::Data(bufferView); | |||
| #elif V8_MAJOR_VERSION >= 8 | |||
| #elif (V8_MAJOR_VERSION > 8 || (V8_MAJOR_VERSION == 8 && V8_MINOR_VERION > 3)) | |||
There was a problem hiding this comment.
This change is not related to Node 19, but I couldn't resist including it. This version check is more correct and I noticed this issue because (if I recall correctly) I was prebuilding node-tree-sitter for some older Electron version which has V8 version 8.0 8.1 or 8.2 and it failed on this line.
| @@ -24,7 +24,7 @@ | |||
| "mocha": "^8.3.1", | |||
| "prebuild": "^10.0.1", | |||
| "superstring": "^2.4.2", | |||
| "tree-sitter-javascript": "https://github.com/tree-sitter/tree-sitter-javascript.git#master" | |||
| "tree-sitter-javascript": "https://github.com/tree-sitter/tree-sitter-javascript.git#936d976a782e75395d9b1c8c7c7bf4ba6fe0d86b" | |||
There was a problem hiding this comment.
newer versions of tree-sitter-javascript fail with an "incompatible language version 14" error. In #127 I upgrade tree-sitter instead.
src/logger.cc
Outdated
| Nan::Call(fn, fn->GetCreationContext().ToLocalChecked()->Global(), 3, argv); | ||
| #else | ||
| Nan::Call(fn, fn->CreationContext()->Global(), 3, argv); | ||
| #endif |
There was a problem hiding this comment.
Just checking - does NAN not provide a wrapper for this API that works across V8 versions?
If not, could you extract a conditionally-defined GetGlobal function from this, which you'd use in all of the changed call sites?
There was a problem hiding this comment.
Doesn't seem like it
https://github.com/search?q=repo%3Anodejs%2Fnan%20CreationContext&type=code
https://github.com/search?q=repo%3Anodejs%2Fnan+GetCreationContext&type=code
I moved it into a function, hope that's what you meant.
verhovsky
force-pushed
the
node-19-smaller
branch
from
1903f17
to
6fc5bb7
Compare
GetCreationContextwas introduced in v8/v8@b38bf5b released as V8 version 9.1 (probably, I'm just looking at thegit tags the commit belongs to and the state of https://github.com/v8/v8/blob/main/include/v8-version.h when the commit is made). Later they added aGetCreationContextCheckedin v8/v8@91475f9 9.7.64 which does the same thing asCreationContext.CreationContextwas removed in commit 46224e7 released as 10.3 (the commit says 10.2 but I think it's lying). So as per this table:node-tree-sitter is trying to use a function that doesn't exist in Node 19+. The oldest supported Electron version, Electron 22 uses V8 10.8.168.25 so this must be a problem there too, but I guess no one is using node-tree-sitter with Electron?
Note that after this commit node-tree-sitter still won't compile on Node 19+ due to the superstring dependency not supporting Node 19 for the exact same reason, but merging this shouldn't break anything.
This PR contains only the most relevant changes from #127