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

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

Closed
mrsimicsak opened this issue Nov 29, 2016 · 8 comments
Closed

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

mrsimicsak opened this issue Nov 29, 2016 · 8 comments

Comments

@mrsimicsak
Copy link

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 ea91305
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
Copy link
Member

z3dev commented Dec 18, 2016

@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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Author

@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 nodejs/node#7165 it looks like a change introduced in 7.0.0.

@kaosat-dev
Copy link
Contributor

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

f90a499

@kaosat-dev
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

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

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

3 participants