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

Remove jQuery dependency #65

Closed
technopagan opened this issue Mar 25, 2018 · 5 comments
Closed

Remove jQuery dependency #65

technopagan opened this issue Mar 25, 2018 · 5 comments

Comments

@technopagan
Copy link

My Javascript skills are beyond abysmal, but yet I think we should tackle the task of removing jQuery from Retromat.

JQuery 1.x and 2.x are officially EOL (jquery/jquery.com#162) and the version that is currently on Retromat (1.7.2) has two known vulnarabilities: https://snyk.io/test/npm/jQuery/1.7.2?severity=high&severity=medium&severity=low

Upgrading to 1.8, 2.x or the currently stable 3.x would take just as much effort as removing the jQuery dependency: Retromat uses only the most basic of jQuery functionalities, most of which should be easily achievable with vanilla JS thanks to the progress that has happened in ECMA5+6.

So I propose that we figure out how to rewrite the bits of jQuery Syntax into vanilla JS and thus remove jQuery. This would save bandwidth, save the last remaining 3rd-party calls to Google and probably even execute faster while removing the known exploits.

devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
devtop pushed a commit to devtop/Retromat that referenced this issue Mar 25, 2018
@fiddike
Copy link
Collaborator

fiddike commented Apr 3, 2018

Hi @technopagan , I understand the idea and the advantages. My JS knowledge is close to zero and I'm not sure I want to add this to my already long learning list. Also, there are two different issues: The XSS vulnerabilities and performance improvements:
Concerning XSS: If I had to take care of this alone, I would probably experiment with jquery 3 combined with jquery-migrate to see if I can get that working. I'd be totally happy about supporting a different strategy, but somebody else would need to take the lead on it.
Concerning performance: For me, personally, the next step towards better performance would be migrating some easier to migrate sites to U7. This way I'll learn about it and get ready to take on the retromat migration to U7, which will hopefully make a difference. Concerning taking jquery out for performance reasons: Same as above: I'd totally happy about to support this, but somebody else would need to take the lead.
@findingmarbles Do you have any experience with upgrading or removing jquery from other projects? Any idea / opinion on how to proceed?

@devtop
Copy link

devtop commented Apr 6, 2018

I don't see any possible vector for XSS at Retromat, since there isn't any unreviewed user content on the site.

I am also wondering, if there actually are any noticable perfomance issues (above 100ms) with jQuery.

And if you would like to spare bandwidth or traffic, at the most activity.json should be divided into smaller parts, Even this is not vital. As long as the activities.json does not grow over 250kb.

Just my 2cents.

@fiddike
Copy link
Collaborator

fiddike commented Apr 6, 2018

Hmm, two guys called Tobias in the conversation, good thing we have nicknames 8-)

devtop: Thank you for your thoughts!
I'm still all for upgrading / removing jQuery, but it's good to know that the vulnerabilities can probably not be exploited in our case.

devtop: Another thing: Github shows commits pushed by you as related to this issue. Just realized it's some old commits from Corinna from before the time when we switched to web based editing of activities. Do you have any idea why github shows them as related to this issue?

@devtop
Copy link

devtop commented Apr 7, 2018

GitHub lists the commits because of their commit message. This issue has the number 65. It is a github feature to reference an issue in a commit message by using hash # + number.

All concerned commit are made by @findingmarbles . She used #+65 reference the activity.

I updated my local repository by pulling it from frindingmarbles/Retromat on Friday, the 23rd, two weeks ago. Then I updated my repository at github by pushing my local repo.

My local repo was long time outdated. That is why the commits are pretty old.

Neat detail: I updated my local repository to see if I could support on this issue. But I am too far away from front end develeopment to help removing jQuery in any kind of way.

@fiddike
Copy link
Collaborator

fiddike commented Apr 7, 2018

@devtop Thank you, now I understand :-D

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

4 participants