search

tree-sitter / node-tree-sitter

Node.js bindings for tree-sitter
https://www.npmjs.com/package/tree-sitter
MIT License
649 stars 114 forks source link

fix: make the native addon context-aware #118

Closed bongnv closed 8 months ago

bongnv commented 1 year ago

Since Electron 14, all native add-ons must be context-aware. Therefore, in order to make node-tree-sitter run with Electron >= 14, it has to be context-aware. Ref: https://github.com/electron/electron/issues/18397

In this PR, NAN_MODULE_WORKER_ENABLED is used to make the native addon context-aware. It will allow context as the third param but will ignore context. Although context param will be accepted, this change doesn't mean that the addon will be safe to be initialised multiple times and can be run safely on worker threads. Perhaps, someone with a better understanding of the library helps to implement AddEnvironmentCleanupHook to clean up global static resources. Ref: https://nodejs.org/api/addons.html#context-aware-addons

Another minor change: xcode_settings settings are consolidated into a single place.

segevfiner commented 10 months ago

It won't really be context aware without also taking care of any static references to JS values the module currently uses.

segevfiner commented 8 months ago

My PR for making it truly context aware was merged, this one can be closed.