jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.
https://openjscad.xyz/
MIT License
2.63k stars 512 forks source link

OpenJSCAD throws fs.js error under node.js 7.0.0 #198

Closed mrsimicsak closed 7 years ago

mrsimicsak commented 7 years ago

When running OpenJSCAD from the command line under node.js 7.0.0 the following error is thrown: note: this is after applying the suggested fix in issue: #197.

~/OpenJSCAD.org$ openjscad examples/example001.jscad converting examples/example001.jscad -> examples/example001.stl (STereoLithography, ASCII) Blob: type [application/sla] size [386566] fs.js:50 throw new TypeError('"options" must be a string or an object, got ' + ^ TypeError: "options" must be a string or an object, got number instead. at getOptions (fs.js:50:11) at Object.fs.writeFile (fs.js:1184:13) at Object. (/usr/local/bin/openjscad:261:4) at Module._compile (module.js:573:32) at Object.Module._extensions..js (module.js:582:10) at Module.load (module.js:490:32) at tryModuleLoad (module.js:449:12) at Function.Module._load (module.js:441:3) at Module.runMain (module.js:607:10) at run (bootstrap_node.js:382:7)

Removing the 0, at the end of line 261 in openjscad fixes the problem.

I suspect this is caused by a node API change because the exact same line works in node v0.10.23, but I haven't been able to find when the API changed.

OpenJSCAD version: ~/OpenJSCAD.org$ git show commit ea91305cf486c88553e03962514c996a9c1c39ef Merge: 8269eff 1b94f54 Author: Z3 Development z3-dev@gfnews.net Date: Sun Sep 11 11:08:52 2016 +0900

Merge pull request #171 from fischman/patch-1

Unbreak intersection() & difference() for CAGs.

node version: ~/OpenJSCAD.org$ node -v v7.0.0

Ubuntu version: ~/OpenJSCAD.org$ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 14.04.3 LTS Release: 14.04 Codename: trusty

z3dev commented 7 years ago

@mrsimicsak Thanks for the information.

There's a major re-organization happening now. Do you think this is important enough for making a patch to 0.5.2?

mrsimicsak commented 7 years ago

The short answer is I don't know.

Scrolling through the Google+ group it seems like I am one of the few (only) people using the command line version which limits the impact of the problem.

I tried applying the same fix to the old copy I have running on node v0.10.23 and it didn't break it, so it doesn't seem like backwards compatibility would be an issue.

I would tend toward releasing a patch, mostly out of concern about new users. The other alternative is to add a known issue comment linking to this issue, and then fix it in the next release.

kaosat-dev commented 7 years ago

@mrsimicsak did you also try under one of the LTS versions of Node ? node.js 7.0.0 has a lot of issues from what I heard Also the way OpenJscad installs itself is not very standard , so that might also mess with how things are behaving. FYI when I run the openjscad CLI directly (NOT using the install, as I have no system wide Node.js, only via NVM) there is not issue for me in 6.9.0

mrsimicsak commented 7 years ago

@kaosat-dev I ended up on version 7.0.0 by installing node and npm and then installing 'n' and running 'sudo n stable'.

I actually wasn't aware that there were LTS versions of node.

Using 'n' to install the LTS version I ended up with version 6.9.2. reverting my change, and running openjscad again I don't have the problem.

I just tried node 7.2.1 and the problem exists in that version as well.

Based on this https://github.com/nodejs/node/pull/7165 it looks like a change introduced in 7.0.0.

kaosat-dev commented 7 years ago

@mrsimicsak thanks for the infos ! this will be fixed together with the huge codebase restructuring , fix was trivial :)

https://github.com/Spiritdude/OpenJSCAD.org/pull/205/commits/f90a4990a7edaa0af5a6f5c0ff79d0861329f1c2

kaosat-dev commented 7 years ago

@mrsimicsak this should be completely fixed, as we have node 7.x as part of the tests on TravisCI, could you please check if it works for you ?

mrsimicsak commented 7 years ago

After installing node v7.10.0 via nvm and the latest cli using the instructions on the github repo page, everything appears to be working fine.

kaosat-dev commented 7 years ago

Ok great , thanks @mrsimicsak ! Closing this, will reopen if needed