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

Upgrade javascript dependencies and remove from being vendored in static/third/ #1709

Closed
timabbott opened this issue Aug 25, 2016 · 7 comments

Comments

@timabbott
Copy link
Sponsor Member

Most of the javascript modules we have vendored under static/third could be fetched via npm with pinned versions just as well (we haven't patched most of them). We should systematically replace as many of these as we can with entries in package.json (and upgrade them to current versions).

When doing so, we should be careful to use git log --follow on the relevant file to make sure Zulip doesn't have any patches not present in upstream (any for any where we do, we should submit them upstream so we can stop maintaining forks of javascript modules!).

@timabbott
Copy link
Sponsor Member Author

We can use a similar approach to 8f73701 for devendoring modules when we're ready to upgrade them.

@timabbott timabbott changed the title Eliminate most vendored javascript under static/third/ Upgrade javascript dependencies and remove from being vendored in static/third/ Nov 2, 2016
@timabbott timabbott added this to the Candidates for next roadmap milestone Nov 2, 2016
@timabbott timabbott modified the milestones: Zulip roadmap, Candidates for next roadmap Nov 18, 2016
@refeed
Copy link
Member

refeed commented Jan 8, 2017

@timabbott I'll work on this issue

refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `winchan.js` from `static/third` and fetch it from `npm`.
- Upgrade `winchan` to 0.2.0.
- Bump up the `PROVISION_VERSION` to 3.21.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `codepointat` from `static/third` and fetch it from `npm`.
- Upgrade `codepointat` to 0.2.0.
- Bump up the `PROVISION_VERSION` to 3.22.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `underscore.js` from `static/third` and fetch it from `npm`.
- Upgrade `underscore.js` to 1.8.3.
- Bump up the `PROVISION_VERSION` to 3.23.

Fixes zulip#1709
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `xdate` from `static/third` and fetch it from `npm`.
- Bump up the `PROVISION_VERSION` to 3.24.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.20.
- Bump up the `PROVISION_VERSION` to 3.25.
- Change some js code to comply with this `autosize` version.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.20.
- Bump up the `PROVISION_VERSION` to 3.25.
- Change some js code to comply with this `autosize` version.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousehweel` to 3.1.13.
- Bump up the `PROVISION_VERSION` to 3.26.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 8, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.
- Bump up the `PROVISION_VERSION` to 3.26.
- Change some js code to comply with this `jquery-mousewheel` version.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 9, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.6.15.
- Bump up the `PROVISION_version` to 3.27.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 9, 2017
- Remove `jquery-form` from `static/third` to `npm`.
- Upgrade `jquery-form` to 3.50.0.
- Bump up the `PROVISION_VERSION` to 3.28.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 10, 2017
- Remove `handlebars.runtime.js` from static/third and fetch it from npm
- Upgrade `handlebars` to 4.0.6.
- Bump up the `PROVISION_VERSION` to 3.28.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 10, 2017
- Remove `handlebars.runtime.js` from static/third and fetch it from npm
- Upgrade `handlebars` to 4.0.6.
- Bump up the `PROVISION_VERSION` to 3.28.

I change the test since there is a patch about line, written in handlebars'
release note:
"Lines containing only block statements and whitespace are now removed."

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 10, 2017
- Remove `handlebars.runtime.js` from static/third and fetch it from npm
- Upgrade `handlebars` to 3.0.3.
- Bump up the `PROVISION_VERSION` to 3.28.

I change the test since there is a patch about line, written in handlebars'
v2.0.0-beta.1 release note:
"Lines containing only block statements and whitespace are now removed."

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 11, 2017
- Remove `winchan.js` from `static/third` and fetch it from `npm`.
- Upgrade `winchan` to 0.2.0.
- Bump up the `PROVISION_VERSION` to 3.21.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 11, 2017
- Remove `codepointat` from `static/third` and fetch it from `npm`.
- Upgrade `codepointat` to 0.2.0.
- Bump up the `PROVISION_VERSION` to 3.22.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 11, 2017
- Remove `underscore.js` from `static/third` and fetch it from `npm`.
- Upgrade `underscore.js` to 1.8.3.
- Bump up the `PROVISION_VERSION` to 3.23.

Fixes zulip#1709
refeed added a commit to refeed/zulip that referenced this issue Jan 11, 2017
- Remove `xdate` from `static/third` and fetch it from `npm`.
- Bump up the `PROVISION_VERSION` to 3.24.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jan 11, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.20.
- Bump up the `PROVISION_VERSION` to 3.25.
- Change some js code to comply with this `autosize` version.

Fixes zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.
- Change some js code to comply with this `jquery-mousewheel` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.21.
- Change some js code to comply with this `autosize` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.
- Change some js code to comply with this `jquery-mousewheel` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.
- Change some js code to comply with this `jquery-mousewheel` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.21.
- Change some js code to comply with this `autosize` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.21.
- Change some js code to comply with this `autosize` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue May 31, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.

Part of zulip#1709.
timabbott pushed a commit to timabbott/zulip that referenced this issue Jun 9, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.
- Bump up the `PROVISION_VERSION` to 5.6.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Changed 'wheelSpeed' in "static/js/emoji_picker.js" from 25 to 0.68
(based on tabbott's testing of scrolling through the emoji list).

Part of zulip#1709.
timabbott pushed a commit that referenced this issue Jun 11, 2017
- Remove `perfect-scrollbar` from `static/third` and fetch it from npm.
- Upgrade `perfect-scrollbar` to 0.7.1.
- Bump up the `PROVISION_VERSION` to 5.6.

Changed `wheelSpeed` in "static/js/scroll_bar.js" to 0.5, because when it
20, the scrollbar scrolls very fast.

Changed 'wheelSpeed' in "static/js/emoji_picker.js" from 25 to 0.68
(based on tabbott's testing of scrolling through the emoji list).

Part of #1709.
refeed added a commit to refeed/zulip that referenced this issue Jul 8, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.21.
- Change some js code to comply with this `autosize` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jul 8, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jul 8, 2017
- Remove `jquery.autosize.js` from `static/third` and fetch it from npm.
- Upgrade `autosize` to 3.0.21.
- Change some js code to comply with this `autosize` version.

Part of zulip#1709.
refeed added a commit to refeed/zulip that referenced this issue Jul 8, 2017
- Remove `jquery-mousewheel` from `static/third` and fetch it from npm.
- Upgrade `jquery-mousewheel` to 3.1.6.

Part of zulip#1709.
@timabbott
Copy link
Sponsor Member Author

Here's some notes on the remaining things that we can usefully make progress on:

  • bootstrap: Needs some significant work to upgrade. Main pain is extracting what changes we've made that aren't to the typeahead; if we can move them to not be in bootstrap code itself, then we can upgrade further via NPM.
  • bootstrap-notify: Has a few small patches we've made. Is a tiny library in general. It's unclear to me whether we still need it -- the last time someone thought about this was like 2013.
  • jquery-idle: Was basically rewritten in ae08f51; given how small it is, it's barely fair to call it "jquery-idle". I think probably we should just clean it up further and move it into static/js/idle.js, properly recording its history.

And these that will probably be in static/third indefinitely:

  • sockjs: Dead upstream AFAIK.
  • marked: A significant fork; we'd need to get extension support into upstream to not fork this.
  • bootstrap-typeahead: Heavily patched and also removed in newer bootstrap upstream (became twitter-typeahead). Not a priority.
  • jquery-filedrop: Significantly patched. Should be replaced by a third-party library that supports chunked uploads to fix Large files fail to upload #9391, so moving to NPM is probably not a good use of time. I'd love to see someone work on this.

timabbott pushed a commit that referenced this issue Dec 13, 2019
- Remove `handlebars.runtime.js` from static/third and fetch it from npm
- Upgrade `handlebars` to 3.0.3.

I change the test since there is a patch about line, written in
handlebars'
v2.0.0-beta.1 release note:
"Lines containing only block statements and whitespace are now removed."

Fixes part of #1709.
@timabbott
Copy link
Sponsor Member Author

There's now just a handful of packages in static/third, most of which are there for a reason. I think we can close this as resolved once we kill bootstrap-notify (Which I think @andersk might do at some point?).

@andersk
Copy link
Member

andersk commented Nov 6, 2020

  • marked: A significant fork; we'd need to get extension support into upstream to not fork this.

Is that really true? I haven’t gone through all our changes in detail, but a cursory glance suggests that we should be able to achieve them using normal JavaScript class inheritance.

Edit: Upon further inspection, it’s not as straightforward as I assumed.

@timabbott
Copy link
Sponsor Member Author

OK, I think this work can all be tracked in other places:

And then there's things we've forked majorly:

  • Upgrading or replacing marked, which should be part of our broader Markdown strategy.
  • Migrating to a more modern typeahead library instead of our bootstrap-typeahead fork.
  • Our jquery-idle is a rewrite.

So I think it makes sense to close this issue as resolved.

@andersk
Copy link
Member

andersk commented May 17, 2021

  • Upgrading bootstrap itself, which we've done a few rounds of since this issue.

I note that Bootstrap 3 was released 2013-08-19, three years before this issue was opened. Although we managed a tiny upgrade from a forked 2.1.0/2.1.1 to a forked 2.3.2 (2013-05-17) seven years late (#16196), we’re still not much closer to upgrading to 3, let alone 4 or 5. This is not by any means a resolved problem.

My view is still that Bootstrap doesn’t do anything for us but get in our way. We have a bunch of CSS whose only purpose is to override Bootstrap’s CSS, and a bunch of CSS that haphazardly extends it, but we’re no longer using it as intended basically anywhere. It seems quite possible that removing Bootstrap entirely would take a similar amount of effort as upgrading it, and would be more worthwhile. But these are both large projects.

  • Migrating to a more modern typeahead library instead of our bootstrap-typeahead fork.

I’m not sure what you meant to imply with the position and wording of this list item, but to be clear, this has not been done—we’re still using our Bootstrap typeahead fork. Since typeahead was removed from Bootstrap 3, this may be an impediment to upgrading Bootstrap, depending on how entangled this code is with the rest of Bootstrap.

You can close issues if it makes you happy I guess, but I’m not going to be happy until our static/third directory is actually deleted. Until then, we have a bunch of critical code that isn’t being maintained either by upstream or by us, and is being excluded from our static analysis tools. An after-the-fact justification for why the particular code we have in static/third is special doesn’t change that fundamental problem.

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

Successfully merging a pull request may close this issue.

4 participants