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

"use strict" in ss_data.js #986

Merged
merged 2 commits into from Nov 25, 2017

Conversation

@edemaine
Copy link
Member

commented Nov 24, 2017

Without this change, I was getting this error message when running sudo texcmp.sh:

/KaTeX/test/screenshotter/ss_data.js:15
let dict = fs.readFileSync(require.resolve("./ss_data.yaml"));
^^^

SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode
@kevinbarabash

This comment has been minimized.

Copy link
Member

commented Nov 24, 2017

@edemaine I don't think I've ever run into this, what version of node are you using?

@edemaine

This comment has been minimized.

Copy link
Member Author

commented Nov 25, 2017

Either 6.11.2 or 6.11.4.

$ node -v
6.11.4
$ sudo node -v
6.11.2

But I think it's more a question of which Node is being used in the docker...

I tried running node texcmp.js directly, and it seems to work in Node 6 and 8, but the error above occurs with Node 4.

Maybe the right solution is to upgrade from Ubuntu 17.04?

@edemaine

This comment has been minimized.

Copy link
Member Author

commented Nov 25, 2017

I confirmed that it's the Docker that's running Node 4:

$ sudo docker create -t -u 0:0 -w /KaTeX/dockers/texcmp katex/texcmp:1.3 nodejs -v
7508fe8ed265d813b8621b17ec33c755f4819bf43cafa633b520226ceb4bb906
$ sudo docker start -a 7508fe8ed265d813b8621b17ec33c755f4819bf43cafa633b520226ceb4bb906
v4.7.2
@edemaine

This comment has been minimized.

Copy link
Member Author

commented Nov 25, 2017

I think we have a few options:

  1. Upgrade to Node 6 or 8. Not sure Ubuntu upgrade would help; I think you're supposed to declare a different package source.
  2. Add --use_strict command-line option to Node
  3. Add "use strict" to this file (as currently in this PR).

Option 1 is maybe best long-term, but 2 (or 3) are simpler fixes. Opinions?

@kevinbarabash

This comment has been minimized.

Copy link
Member

commented Nov 25, 2017

@edemaine thanks for investigating. I think we should do 1 long term. In the short term let's merge this. Maybe it was working for me before because we didn't use let before. 🤔

@kevinbarabash kevinbarabash merged commit c103693 into KaTeX:master Nov 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.