Upgrade the source to use ES6 syntax including classes, import and static properties #679

HosseinAgha · 2017-04-09T17:09:33Z

Discussed at #654.
I also think this fixes the second issue in the project's backlog here.

… using ES6 classes)

khanbot · 2017-04-09T17:09:41Z

CLA signature looks good 👍

kevinbarabash · 2017-04-16T01:01:28Z

.eslintrc

@@ -14,7 +15,7 @@
        "indent": [2, 4, {"SwitchCase": 1}],
        "keyword-spacing": 2,
        "linebreak-style": [2, "unix"],
-        "max-len": [2, 80, 4, { "ignoreUrls": true, "ignorePattern": "\\brequire\\([\"']|eslint-disable" }],
+        "max-len": [2, 90, 4, { "ignoreUrls": true, "ignorePattern": "\\brequire\\([\"']|eslint-disable" }],


why the change in max length?

Answering my own question: because of the extra indent. I'm fine with this.

My two cents: I'm a fan of limiting to 80 columns. All my terminal windows are that wide, thanks to good old punch cards...

@HosseinAgha how much of the code would be affected by keeping the 80 column limit?

@kevinbarabash Adding classes adds about 4 spaces to each line. I can decrease it to 84 but a number less than that makes a lot of lint errors.
most of the Parser code (about 13 lines) would have error for example.

kevinbarabash · 2017-04-16T01:02:08Z

.gitignore

@@ -4,6 +4,7 @@ npm-debug.log
 last.png
 diff.png
 /.npm-install.stamp
+/.vscode


could add this to your global .gitignore?

@kevinbarabash sorry I missed that one. Fixed.

kevinbarabash · 2017-04-16T01:06:05Z

@HosseinAgha cool! Thanks for taking this on.

kevinbarabash · 2017-04-16T01:07:32Z

Thanks for keeping the constant definitions close to their use site.

kevinbarabash · 2017-04-16T01:10:39Z

@HosseinAgha after you resolve this conflict this should be good to go.

HosseinAgha · 2017-04-16T12:19:39Z

Thank you all the KaTeX team specially @kevinbarabash for all your help and patiently answering questions.

HosseinAgha · 2017-04-16T14:21:24Z

@kevinbarabash All Done.

pouriaMaleki · 2017-05-22T10:21:12Z

How it's going? @kevinbarabash

kevinbarabash · 2017-05-23T01:18:26Z

I forgot about this PR :(. I'd like to land #670 first b/c I think it's going to be less work to rebase this on to those changes than the other way around. @HosseinAgha sorry about not merging this sooner and avoid extra work for you.

kevinbarabash · 2017-06-29T22:49:41Z

@HosseinAgha #670 has been merged. As a result there are lots of conflicts. :sad: Would you be willing to rebase this? If not, no worries. I have some and come open a fresh PR for this.

HosseinAgha · 2017-06-30T10:44:03Z

@kevinbarabash Sure, if you did not do anything yet.
I had problems with server.js file you have a babel and none babel endpoint for it. Is it OK to change it to only use babel transpiled files?
I will start merging as soon as you give a OK here.

kevinbarabash · 2017-06-30T17:22:51Z

@HosseinAgha awesome! What kind of issues are you seeing with server.js?

HosseinAgha · 2017-07-01T12:51:06Z

@kevinbarabash I've described the problem at the end of
this issue

kevinbarabash · 2017-07-01T16:59:58Z

@HosseinAgha thanks or refreshing my memory. I don't think we have much of choice here. Could you make a separate PR for the changes to server.js?

HosseinAgha · 2017-07-02T10:32:16Z

@kevinbarabash OK

HosseinAgha · 2017-07-02T16:24:38Z

@kevinbarabash I created the PR (#756) for server.js fix. I working on fixing conflicts now.

HosseinAgha · 2017-07-02T17:32:16Z

@kevinbarabash everything is merged and all tests are passing.
Could you please consider merging this before any new merge? because fixing merge conflicts in this scale is a bit time consuming and git is a bit dumb in this case. Thanks.

kevinbarabash

Requesting changes to server.js before this can proceed.

kevinbarabash · 2017-07-02T18:39:19Z

server.js

@@ -47,8 +47,9 @@ function serveBrowserified(file, standaloneName, doBabelify) {
 }

 function twoBrowserified(url, file, standaloneName) {


Can you change the name to browserified?

kevinbarabash · 2017-07-02T18:40:14Z

server.js

-    app.get("/babel" + url, serveBrowserified(file, standaloneName, true));
+    // we should always transpile all js files
+    // in order to support all ES2015 features
+    app.get(url, serveBrowserified(file, standaloneName, true));
 }

 function twoUse(url, handler) {


We don't need the /babel endpoints that this function generates either. Can you replace all calls to twoUse with app.use?

kevinbarabash · 2017-07-02T18:42:48Z

.eslintrc

@@ -14,7 +15,7 @@
        "indent": [2, 4, {"SwitchCase": 1}],
        "keyword-spacing": 2,
        "linebreak-style": [2, "unix"],
-        "max-len": [2, 80, 4, { "ignoreUrls": true, "ignorePattern": "\\brequire\\([\"']|eslint-disable" }],
+        "max-len": [2, 84, 4, { "ignoreUrls": true, "ignorePattern": "\\brequire\\([\"']|eslint-disable" }],


I'm okay with this for this commit, but we should revert it afterwards. How do other people feel?

kevinbarabash · 2017-07-02T18:50:58Z

src/Parser.js

    }
-};
+}

 Parser.prototype.ParseNode = ParseNode;


Within the class you could have a line ParseNode = ParseNode. This is similar to static ParseNode = ParseNode but adds the value to the prototype instead of the class itself.

kevinbarabash · 2017-07-02T18:54:02Z

src/Lexer.js

@@ -84,26 +79,36 @@ const tokenRegex = new RegExp(
    ")"
 );

-/**
- * This function lexes a single token.
+/*


super minor nit: /**

kevinbarabash · 2017-07-03T00:34:23Z

I just realized that screenshotter also uses /babel endpoint. Can you update screenshotter.js to not use the /babel endpoint?

…g now

HosseinAgha · 2017-07-03T06:38:35Z

All Done

kevinbarabash · 2017-07-03T12:15:47Z

@HosseinAgha thanks for the PR. I'm excited about using ES6 in our codebase.

HosseinAgha added 7 commits March 21, 2017 16:48

Add babel transform-class-properties to have static class properties

130e9f9

Upgrade Lexer and Parser files to use ES6 classes

a4561cc

Update eslint max line length to 90 character (more indent because of…

add0354

… using ES6 classes)

Upgrade eslint and jasmin to support ES stage-2 features

7413d7e

Use static properties to place constants near their functions

7f05366

Migrate all remaining sources to ES6 syntax

2953f54

Merge with master

d7a849b

kevinbarabash reviewed Apr 16, 2017

View reviewed changes

HosseinAgha added 2 commits April 16, 2017 17:52

Decrease eslint max line length to 84

e844db1

merge

c7b8d30

HosseinAgha changed the title ~~Upgrade the source to use ES6 classes, import, static syntax~~ Upgrade the source to use ES6 classes, import, static properties and syntax Apr 16, 2017

HosseinAgha changed the title ~~Upgrade the source to use ES6 classes, import, static properties and syntax~~ Upgrade the source to use ES6 syntax including classes, import and static properties Apr 16, 2017

Remove non-babelified endpoint in dev server.js

0b4eaf4

HosseinAgha mentioned this pull request Jul 2, 2017

Remove endpoint with not transpiled js in dev server.js #756

Closed

Major Merge to add ES6 features

6f1abc1

kevinbarabash requested changes Jul 2, 2017

View reviewed changes

kevinbarabash added the GH Review: needs-revision label Jul 3, 2017

kevinbarabash mentioned this pull request Jul 3, 2017

Make KaTeX work in Quirks mode #608

Merged

HosseinAgha added 2 commits July 3, 2017 11:06

Clean up server.js functions after removing browserified

e0fac36

Make screenshotter not to use babel endpoint as we babelify everythin…

15bbbbf

…g now

kevinbarabash added GH Review: review-needed and removed GH Review: needs-revision labels Jul 3, 2017

kevinbarabash approved these changes Jul 3, 2017

View reviewed changes

kevinbarabash merged commit a019f36 into KaTeX:master Jul 3, 2017

kevinbarabash added GH review: accepted and removed GH Review: review-needed labels Jul 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade the source to use ES6 syntax including classes, import and static properties #679

Upgrade the source to use ES6 syntax including classes, import and static properties #679

HosseinAgha commented Apr 9, 2017 •

edited

Loading

khanbot commented Apr 9, 2017

kevinbarabash Apr 16, 2017

kevinbarabash Apr 16, 2017

edemaine Apr 16, 2017

kevinbarabash Apr 16, 2017

HosseinAgha Apr 16, 2017 •

edited

Loading

kevinbarabash Apr 16, 2017

HosseinAgha Apr 16, 2017 •

edited

Loading

kevinbarabash commented Apr 16, 2017

kevinbarabash commented Apr 16, 2017

kevinbarabash commented Apr 16, 2017

HosseinAgha commented Apr 16, 2017 •

edited

Loading

HosseinAgha commented Apr 16, 2017 •

edited

Loading

pouriaMaleki commented May 22, 2017

kevinbarabash commented May 23, 2017

kevinbarabash commented Jun 29, 2017

HosseinAgha commented Jun 30, 2017 •

edited

Loading

kevinbarabash commented Jun 30, 2017

HosseinAgha commented Jul 1, 2017 •

edited

Loading

kevinbarabash commented Jul 1, 2017

HosseinAgha commented Jul 2, 2017

HosseinAgha commented Jul 2, 2017 •

edited

Loading

HosseinAgha commented Jul 2, 2017 •

edited

Loading

kevinbarabash left a comment

kevinbarabash Jul 2, 2017

HosseinAgha Jul 3, 2017

kevinbarabash Jul 2, 2017

kevinbarabash Jul 2, 2017

kevinbarabash Jul 2, 2017

kevinbarabash Jul 2, 2017

kevinbarabash commented Jul 3, 2017

HosseinAgha commented Jul 3, 2017

kevinbarabash commented Jul 3, 2017

		@@ -47,8 +47,9 @@ function serveBrowserified(file, standaloneName, doBabelify) {
		}

		function twoBrowserified(url, file, standaloneName) {

Upgrade the source to use ES6 syntax including classes, import and static properties #679

Upgrade the source to use ES6 syntax including classes, import and static properties #679

Conversation

HosseinAgha commented Apr 9, 2017 • edited Loading

khanbot commented Apr 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HosseinAgha Apr 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HosseinAgha Apr 16, 2017 • edited Loading

Choose a reason for hiding this comment

kevinbarabash commented Apr 16, 2017

kevinbarabash commented Apr 16, 2017

kevinbarabash commented Apr 16, 2017

HosseinAgha commented Apr 16, 2017 • edited Loading

HosseinAgha commented Apr 16, 2017 • edited Loading

pouriaMaleki commented May 22, 2017

kevinbarabash commented May 23, 2017

kevinbarabash commented Jun 29, 2017

HosseinAgha commented Jun 30, 2017 • edited Loading

kevinbarabash commented Jun 30, 2017

HosseinAgha commented Jul 1, 2017 • edited Loading

kevinbarabash commented Jul 1, 2017

HosseinAgha commented Jul 2, 2017

HosseinAgha commented Jul 2, 2017 • edited Loading

HosseinAgha commented Jul 2, 2017 • edited Loading

kevinbarabash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinbarabash commented Jul 3, 2017

HosseinAgha commented Jul 3, 2017

kevinbarabash commented Jul 3, 2017

HosseinAgha commented Apr 9, 2017 •

edited

Loading

HosseinAgha Apr 16, 2017 •

edited

Loading

HosseinAgha Apr 16, 2017 •

edited

Loading

HosseinAgha commented Apr 16, 2017 •

edited

Loading

HosseinAgha commented Apr 16, 2017 •

edited

Loading

HosseinAgha commented Jun 30, 2017 •

edited

Loading

HosseinAgha commented Jul 1, 2017 •

edited

Loading

HosseinAgha commented Jul 2, 2017 •

edited

Loading

HosseinAgha commented Jul 2, 2017 •

edited

Loading