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

Installing mathsteps is not straightforward #77

Closed
kovzol opened this issue Jan 20, 2017 · 7 comments · Fixed by #79
Closed

Installing mathsteps is not straightforward #77

kovzol opened this issue Jan 20, 2017 · 7 comments · Fixed by #79
Labels

Comments

@kovzol
Copy link

kovzol commented Jan 20, 2017

I tried several ways to install mathsteps on Ubuntu Linux 16.04.1 LTS with just partial success.

First method: "sudo apt install nodejs". It installs node 4.2.6 (which is quite old). Then by running "npm install mathsteps" a folder node_modules is created, it also includes the mathsteps/ folder. Now I tried to create a file with the example .js code with name test.js, but I get the following error when running it by "nodejs test.js":

/home/kovzol/workspace/node_modules/mathsteps/lib/simplifyExpression/index.js:6
function simplifyExpressionString(expressionString, debug=false) {
                                                         ^

SyntaxError: Unexpected token =
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:374:25)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/home/kovzol/workspace/node_modules/mathsteps/index.js:1:90)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)

Second method: I built mathsteps as described on the main page. Then I copied the lib folder over the folder where the first method created mathsteps' lib folder. But I got the same error.

Third method: I downloaded newer node versions from its website. The I started "nodejs test.js" by explicitly using the correct path to the different node versions. I got the following for node version 6.9.4:

/home/kovzol/workspace/mathsteps-test/test.js:4
  console.log(step.oldNode.toString());    // "2 x + 2 x + x + x"
                          ^

TypeError: Cannot read property 'toString' of null
    at steps.forEach.step (/home/kovzol/workspace/mathsteps-test/test.js:4:27)
    at Array.forEach (native)
    at Object.<anonymous> (/home/kovzol/workspace/mathsteps-test/test.js:3:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:394:7)

and also the same for node version 7.4.0. It seems that step.oldNode does not exists. If I remove that line, I can make the rest work:

ADD_POLYNOMIAL_TERMS
6 x
3
@hmaurer
Copy link
Contributor

hmaurer commented Jan 20, 2017

For the former, it is because "default parameters" in Javascript (an ES6 feature) were not enabled by default before node 6.0.0 (I think). So you need to be running at least node 6.

For the latter, what is the content of test.js? It does not seem to be a file at the root of the repository; did you create it?

@hmaurer
Copy link
Contributor

hmaurer commented Jan 20, 2017

Ah nevermind, I guess you copied the example script from the README into a test.js file. As far as I can tell there is either a mistake in the example or in the codebase. The property oldNode is null in the first step, however it is present in all the substeps. @evykassirer

@hmaurer
Copy link
Contributor

hmaurer commented Jan 20, 2017

After taking a look it seems it's due to some weirdness with Status.reset. When calling nodeStatus.reset() here it sets the oldNode property of the object that has just been pushed to steps to null.

I suspect this bug was not present at the time of this commit because it was manually cloning the node status. This might explain why the example does not reflect what's actually going on :-)

@evykassirer
Copy link
Contributor

evykassirer commented Jan 21, 2017

  1. Yeah, you need at least node 6 for default parameters. I'll open an issue to add that to the documentation.

  2. Okay yeah that's our fault. You have it installed correctly. We'll fix that bug and release a new version on npm and close this issue only once that's fixed

Thanks so much @kovzol for bringing both these issues up and @hmaurer for investigating 😄

@kovzol
Copy link
Author

kovzol commented Jan 21, 2017

Thank you for the prompt reply!

@evykassirer
Copy link
Contributor

(fix made, reopening until I publish a new version of npm)

@evykassirer
Copy link
Contributor

should be fixed now! please re-open this issue if you have any further problems with this.

thanks again for letting us know and @hmaurer and @Raibaz for fixing 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants