mo4islona / node-blockly

Blockly for Node.js and Browser via CommonJS module
135 stars 81 forks source link

current node-blockly NPM not installing on NodeJS 10.x #28

Closed eyz closed 5 years ago

eyz commented 6 years ago

node-blockly does not install on NodeJS 10.x

I have a workaround PR at znerol/node-xmlshim#8 to use libxmljs 0.19.3 (PR libxmljs/libxmljs#523), which supports NodeJS 10.x. Unfortunately there are multiple dependencies involved for the workaround, until znerol/node-xmlshim considers the aforementioned PR to use libxmljs 0.19.3+

My fork at eyz/node-blockly@3dd2524c1b222cbb00817266f12a40e5a1654cef implements these changes for my own personal use, but I believe the proper solution is for znerol/node-xmlshim#8 to be evaluated and potentially merged

mo4islona commented 6 years ago

I've tried your fork

 "xmlshim": "git://github.com/eyz/node-xmlshim.git#be6220b53809b2c5a84be25c17e56bc4a12ba411"

Everything seems to be ok, but some tests are failed

  13 passing (151ms)
  2 failing

  1) XML
       domToText:
     TypeError: xml.TextWriter is not a constructor
      at exports.XMLSerializer.serializeToString (node_modules/xmlshim/index.js:24:22)
      at Object.module.exports.Blockly.Xml.domToText (lib/blockly_compressed.js:1279:450)
      at Context.<anonymous> (test/xml_test.js:43:30)

  2) XML
       domToPrettyText:
     TypeError: xml.TextWriter is not a constructor
      at exports.XMLSerializer.serializeToString (node_modules/xmlshim/index.js:24:22)
      at Object.module.exports.Blockly.Xml.domToText (lib/blockly_compressed.js:1279:450)
      at Object.module.exports.Blockly.Xml.domToPrettyText (lib/blockly_compressed.js:1280:55)
      at Context.<anonymous> (test/xml_test.js:50:30)

xml.TextWriter is missing. Any ideas?

eyz commented 6 years ago

@mo4islona I'm not having any problems with this fork in my project, but yes -- I am able to reproduce those errors with the following (minimum reproduction from the pertinent code in xmlshim/index.js) -

var xml = require("libxmljs");
var textwriter = new xml.TextWriter();

I suspect this is why there was a workaround before; see the original git://github.com/znerol/libxmljs.git#xmlwriter-0.18.0 that I replaced with the latest libxmljs, at https://github.com/eyz/node-xmlshim/commit/be6220b53809b2c5a84be25c17e56bc4a12ba411

I think the best answer here is for znerol/libxmljs to take a look at the NodeJS 10.x support, and see if there may need to be a variation of that workaround for NodeJS 10.x.

It may be that I simply don't use that code path, and haven't come across this issue myself. Do we know how to exercise this failing code path with node-blocky?

dmythro commented 5 years ago

So, any updates on this or everyone is on Node 8.x or older?

eyz commented 5 years ago

@z-ax I'm on Node 10.x on my fork (top comment, above), but I'm quite certain I'm not using all code paths in node-blockly

dmythro commented 5 years ago

@eyz Me neither, but npm install fails because of one of it's sub-dependencies.

znerol commented 5 years ago

I did just released xmlshim 0.2.4 which is compatible with node 10.

mo4islona commented 5 years ago

@znerol :(

"dependencies": {
    "jsdom": "^14.0.0",
    "xmlshim": "^0.2.5"
}

node -v v10.15.2

gulp[48582]: ../src/node_contextify.cc:633:static void node::contextify::ContextifyScript::New(const FunctionCallbackInfo<v8::Value> &): Assertion `args[1]->IsString()' failed.
 1: 0x10003860e node::Abort() [/usr/local/Cellar/node@10/10.15.2/bin/node]
 2: 0x100037748 node::AddEnvironmentCleanupHook(v8::Isolate*, void (*)(void*), void*) [/usr/local/Cellar/node@10/10.15.2/bin/node]
 3: 0x10005c44b node::contextify::ContextifyScript::New(v8::FunctionCallbackInfo<v8::Value> const&) [/usr/local/Cellar/node@10/10.15.2/bin/node]
 4: 0x1001cfb19 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) [/usr/local/Cellar/node@10/10.15.2/bin/node]
 5: 0x1001cee2f v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<true>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/usr/local/Cellar/node@10/10.15.2/bin/node]
 6: 0x1001ce8b0 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/usr/local/Cellar/node@10/10.15.2/bin/node]
 7: 0x21b8c4f5be3d 
/var/folders/n1/0vlb066d7md10j1jckns_3dh0000gn/T/yarn--1553549475371-0.5941561074139992/node: line 3: 48582 Abort trap: 6           "/usr/local/Cellar/node@10/10.15.2/bin/node" "$@"
error Command failed with exit code 134.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
znerol commented 5 years ago

@mo4islona this stack trace doesn't look like it is related to jsdom/xmlshim. The one in gulpjs/gulp#2162 looks quite similar though.

mo4islona commented 5 years ago

@znerol Thanks. Now it supports NodeJS 10.x and later

eyz commented 5 years ago

Thanks, folks!