Skip to content

Conversation

@oldnomad
Copy link
Contributor

@oldnomad oldnomad commented Jul 9, 2021

This is a re-implementation of getRelativeDate(), which was mentioned in #1433. Original version used NodeJS module moment.js, which is obsolete, deprecated, and slated for removal from Nextcloud base.

This version uses pure JavaScript Intl.RelativeTimeFormat, with fallback to plain date formatting for older browsers. Dependency on moment.js is dropped. The new function is, however, slightly different from the original:

  • If the distance between now and the timestamp is more than 90 days (and for older browsers), absolute date/time format is used. I didn't want to use "N months ago" relative format, since it's difficult to calculate correctly; and "N weeks ago" seems to be less readable for long distances.
  • The function returns "N weeks ago" for distances greater than 7 days, "N days ago" for distances greater than 1 day, and so on.
  • In the fallback (absolute format) branch the function returns time only to minutes. If the timestamp is within half-day from now, only time is printed; if the timestamp is within 7 days from now, weekday is added; otherwise, full date is added.

My last encounter with JavaScript was about twenty years ago, so any corrections or improvements are welcome.

@SMillerDev
Copy link
Contributor

Do you also have any idea why the functionality is called as often as it is? Because it seems to be called excessively right?

@anoymouserver
Copy link
Contributor

Thanks for the PR! I'll try to test it as soon as possible and report back how it works.

Apparently NC itself uses @nextcloud/moment for date formatting (I've checked files, polls, mail) but im not sure how well this performs in this case.
But maybe it's easier to maintain in the future than a custom implementation.

Please also add an entry under the 'Unreleased' section in the CHANGELOG.md and follow the instructions here for signing off the commit: https://github.com/nextcloud/news/pull/1441/checks?check_run_id=3032523832

@oldnomad
Copy link
Contributor Author

oldnomad commented Jul 9, 2021

Fixed the sign-off and the changelog. Also, small speed improvement in the fallback branch (no need to create a format object).

@oldnomad
Copy link
Contributor Author

Apparently NC itself uses @nextcloud/moment for date formatting (I've checked files, polls, mail) but im not sure how well this performs in this case.

@nextcloud/moment is just a thin wrapper over the same momentjs. There was an issue about replacing the underlying library (nextcloud-libraries/nextcloud-moment/issues/407), but no solution.

For the status of momentjs (Project Status), its maintainers state that the project is not developed any more and recommend some alternatives.

@oldnomad
Copy link
Contributor Author

Do you also have any idea why the functionality is called as often as it is? Because it seems to be called excessively right?

Since I'm not a specialist in JS, I can only guess. From what I've seen in the code. getRelativeDate() is called every time corresponding ng-template is expanded, but as to what causes this expansion, I can only guess.

I've tried counting calls on some small feeds. On a feed with 1 article, there were 6 calls during page reload, and then 9 more calls when I clicked on the article. On a feed with 3 articles, there were 18 calls during page reload, and then 37 (!!!) more when I clicked on the first article. After that, with remaining 2 articles I got 12 calls on reload and 21 calls on click. So, my initial guess is that for n articles we get 6*n calls on reload, and 2*n*n + 6*n + 1 calls for click. For, say, 100 articles that means 20601 calls on click, which would definitely take some time. And I have some folders with more than 1000 articles.

@oldnomad
Copy link
Contributor Author

So, my initial guess is that for n articles we get 6*n calls on reload, and 2*n*n + 6*n + 1 calls for click.

Seems that the formula is right, with some caveats: on a feed with 156 articles I get 240 calls on reload and 3441 on click (as if there were only 40 articles), but once I scroll down to the end of the feed (26656 calls), click on an article now takes 51166 calls.

@oldnomad
Copy link
Contributor Author

I am abandoning this PR in favor of #1450.

Implementing relative date formatting as a filter, as in #1450 seems to solve excessive calls problem completely.

@oldnomad oldnomad closed this Jul 16, 2021
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

Successfully merging this pull request may close these issues.

3 participants