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

Plans to replace moment.js? #407

Open
raimund-schluessler opened this issue Apr 21, 2021 · 12 comments
Open

Plans to replace moment.js? #407

raimund-schluessler opened this issue Apr 21, 2021 · 12 comments
Labels
question Further information is requested

Comments

@raimund-schluessler
Copy link

Since moment.js is not really developed further (see https://momentjs.com/docs/#/-project-status/) I wonder whether there are plans to replace moment with an actively developed and maybe more modern alternative? On the moment.js website they e.g. mention date-fns https://date-fns.org/. This seems like a nice library, should be tree-shakeable, and especially the immutability of the date objects might help writing clean code.

Any plans and opinions?

@raimund-schluessler raimund-schluessler added the question Further information is requested label Apr 21, 2021
@ChristophWurst
Copy link
Contributor

👍 on replacing the lib if moment is abandoned

As this package is a wrapper of moment we might want to start a new wrapper of date-fns that initalizes the lib with Nextcloud's locale/language settings and later mark this package as deprecate with a notice for people to use the new package.

@ChristophWurst
Copy link
Contributor

@oldnomad would it make sense to package your code from nextcloud/news#1450 into something like a @nextcloud/relative-date as a successor of this package. It would still integrate with Nextcloud and detect the user locale/language like this wrapper does.

@oldnomad
Copy link

@oldnomad would it make sense to package your code from nextcloud/news#1450 into something like a @nextcloud/relative-date as a successor of this package. It would still integrate with Nextcloud and detect the user locale/language like this wrapper does.

I'm not sure whether this code makes sense as a generic package. It contains some arbitrary decisions on where to switch formats; also, a fallback for older browsers which might be not very relevant any more.

Some sort of generic "relative time" formatting for Nextcloud apps would be beneficial, but I think someone more knowledgeable in the technologies used here will take the task. I have not touched JavaScript for more than two decades, and took this task out of desperation.

@TeaDrinkingProgrammer
Copy link

Is this issue still active? I have done a bit of work on trying to make locales work on calendar, and I have a few ideas about how we could go about this:

  1. Since Moment is now used, Luxon might be a better fit for replacing it. Luxon uses roughly the same API, but it uses the Intl API and it is immutable which makes it way easier to work with.
  2. The Nextcloud Luxon API could be incredibly simple: it could just take the user settings, put them in a constant, and give that to the user. I quickly put together and tested this example:
import * as luxon from 'luxon'
// import { getLocale } from '@nextcloud/l10n'
const getLocale = () => {
    return 'es-ar'
}

luxon.Settings.defaultLocale = getLocale();

export default luxon

It would then be used as follows:

import luxon from ".";

// Test. Outputs the following: 17 de sep. de 2023 13:04
console.log(luxon.DateTime.now().toLocaleString(luxon.DateTime.DATETIME_MED))

To make this work well though, I think we should simultaneously tackle the issue with the Locale settings. Right now, a user can pick any language with any locale, when they should ideally only be allowed to pick existing locales. Luckily, this is made simpler since we can simply only allow Intl locales since they should all work per definition.

@TeaDrinkingProgrammer
Copy link

I have also done some research on the other alternatives to momentjs, and this is what I found:

Date-fns:

Advantages: faster and more popular. It is tree-shakable and can thus be very small.
Disadvantage: It uses a format which is very different to Momentjs. It uses their own locales
My conclusion: Uses a format different to Momentjs, would require a bit more refactoring than Luxon: consider

Dayjs

Advantages: Very tiny, 1-to-1 compatible with Momentjs-syntax
Disadvantages: Uses their own locales. Has a very basic feature-set. Is less active than date-fns and Luxon.
My conclusion: Nextcloud apps probably need a lot of features: do not use.

Luxon

Advantages: Is made by the makers of Momentjs. Suppposedly has better time-zone support than date-fns. Uses Intl for time-zones and locales
Disadvantages: No tree-shaking, large package size. Less popular than date-fns

From what I gather, Luxon is a bit more robust and feature-complete than date-fns and the reason most people use date-fns is because most websites don't make extensive use of all the features in it. With, Nextcloud this is not the case, so my preference goes to Luxon for now. Also, tree-shaking might be less of a problem if we use most of the library anyway. That being said, I have only just looked around on the internet a bit, so I might be wrong.

Interesting reads:
https://www.npmjs.com/package/eslint-plugin-you-dont-need-momentjs#string--date-format
https://github.com/dmtrKovalenko/date-io/tree/master/packages/benchmark
mui/material-ui-pickers#61

@raimund-schluessler
Copy link
Author

I would vote for date-fns. I think tree-shaking is an important feature these days as many apps will only use a small subset of the features available. Not every app is a calendar and some only use momentjs to e.g. show a timestamp with the correct locale.

Since we anyway will wrap the library selected, we can work around or implement missing features or map the locales available.

@TeaDrinkingProgrammer
Copy link

Fair enough. The only thing that concerns me then is the relatively little amount of languages/locales that date-fns has.
https://github.com/date-fns/date-fns/tree/main/src/locale
vs
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl#locales_argument
https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry

That being said, there is no gaurantee that all subtags are supported.

@TeaDrinkingProgrammer
Copy link

TeaDrinkingProgrammer commented Sep 23, 2023

I did some more research to see what locales actually are missing from date-fns as compared to momentjs. It seems that most missing locales are local variants of languages (Singapore English) or more rare languages (Nepali, Breton). The only two that really stood out are Swahili and Tagalog/Filipino. Also, apparently someone made a Klingon locale, yes really. All in all, I think that locales are thus not much of an obstacle and if they turn out to be we can point the person that is missing the locale to date-fns to contribute to it, as it doesn't seem that much work to add a locale.

In other words: go ahead with date-fns?

date-fns locales:

["af","ar","ar-dz","ar-eg","ar-ma","ar-sa","ar-tn","az","be","be-tarask","bg","bn","bs","ca","cs","cy","da","de","de-at","el","en-au","en-ca","en-gb","en-ie","en-in","en-nz","en-us","en-za","eo","es","et","eu","fa-ir","fi","fr","fr-ca","fr-ch","fy","gd","gl","gu","he","hi","hr","ht","hu","hy","id","is","it","it-ch","ja","ja-hira","ka","kk","km","kn","ko","lb","lt","lv","mk","mn","ms","mt","nb","nl","nl-be","nn","oc","pl","pt","pt-br","ro","ru","sk","sl","sq","sr","sr-latn","sv","ta","te","th","tr","ug","uk","uz","uz-cyrl","vi","zh-cn","zh-hk","zh-tw"]

momentjs locales:

["en","af","ar-dz","ar-kw","ar-ly","ar-ma","ar-sa","ar-tn","ar","az","be","bg","bm","bn-bd","bn","bo","br","bs","ca","cs","cv","cy","da","de-at","de-ch","de","dv","el","en-au","en-ca","en-gb","en-ie","en-il","en-in","en-nz","en-sg","eo","es-do","es-mx","es-us","es","et","eu","fa","fi","fil","fo","fr-ca","fr-ch","fr","fy","ga","gd","gl","gom-deva","gom-latn","gu","he","hi","hr","hu","hy-am","id","is","it-ch","it","ja","jv","ka","kk","km","kn","ko","ku","ky","lb","lo","lt","lv","me","mi","mk","ml","mn","mr","ms-my","ms","mt","my","nb","ne","nl-be","nl","nn","oc-lnc","pa-in","pl","pt-br","pt","ro","ru","sd","se","si","sk","sl","sq","sr-cyrl","sr","ss","sv","sw","ta","te","tet","tg","th","tk","tl-ph","tlh","tr","tzl","tzm-latn","tzm","ug-cn","uk","ur","uz-latn","uz","vi","x-pseudo","yo","zh-cn","zh-hk","zh-mo","zh-tw"]

Missing locales:

["en","ar-kw","ar-ly","bm","bn-bd","bo","br","cv","de-ch","dv","en-il","en-sg","es-do","es-mx","es-us","fa","fil","fo","ga","gom-deva","gom-latn","hy-am","jv","ku","ky","lo","me","mi","ml","mr","ms-my","my","ne","oc-lnc","pa-in","sd","se","si","sr-cyrl","ss","sw","tet","tg","tk","tl-ph","tlh","tzl","tzm-latn","tzm","ug-cn","ur","uz-latn","x-pseudo","yo","zh-mo"]

Missing locales with full names (from script with missing values filled in by hand):

["English","Arabic-Kuwait","Arabic-Libya","Bambara","Bengali-Bangladesh","Tibetan","Breton","Chuvash","German-Switzerland","Divehi","English-Israel","English-Singapore","Spanish-Dominican Republic","Spanish-Mexico","Spanish-United States","Persian","Filipino","Faroese","Irish","Konkani - Devanagari script","Konkani - Latin script","Armenian-Armenia","Javanese","Kurdish","Kyrgyz","Lao","Montenegrin","Māori","Malayalam","Marathi","Malay-Malaysia","Burmese","Nepali","Occitan - lengadocian dialect","Panjabi-India","Sindhi","Northern Sami","Sinhala","Serbian-Cyrillic script","Swati","Swahili","Tetun Dili (East Timor)","Tajik","Turkmen","Tagalog-Philippines","Klingon","Talossan","Tamazight-Latin script","Tamazight","Uyghur-China","Urdu","Uzbek-Latin script","x-pseudo","Yoruba","Chinese-Macao"]

The script:

const fs = require('fs');
const logger = require('pino')()
const ISO6391 = require('iso-639-1');

var date_fns_locales = fs.readdirSync('./node_modules/date-fns/locale');
date_fns_locales.splice(0, 1);
date_fns_locales = date_fns_locales.filter((item) => item !== "index.js" && item !== "index.js.flow" && item !== "package.json" && item !== "types.js");
date_fns_locales = date_fns_locales.map((item) => item.toLowerCase())
const momentLocales = ["en","af","ar-dz","ar-kw","ar-ly","ar-ma","ar-sa","ar-tn","ar","az","be","bg","bm","bn-bd","bn","bo","br","bs","ca","cs","cv","cy","da","de-at","de-ch","de","dv","el","en-au","en-ca","en-gb","en-ie","en-il","en-in","en-nz","en-sg","eo","es-do","es-mx","es-us","es","et","eu","fa","fi","fil","fo","fr-ca","fr-ch","fr","fy","ga","gd","gl","gom-deva","gom-latn","gu","he","hi","hr","hu","hy-am","id","is","it-ch","it","ja","jv","ka","kk","km","kn","ko","ku","ky","lb","lo","lt","lv","me","mi","mk","ml","mn","mr","ms-my","ms","mt","my","nb","ne","nl-be","nl","nn","oc-lnc","pa-in","pl","pt-br","pt","ro","ru","sd","se","si","sk","sl","sq","sr-cyrl","sr","ss","sv","sw","ta","te","tet","tg","th","tk","tl-ph","tlh","tr","tzl","tzm-latn","tzm","ug-cn","uk","ur","uz-latn","uz","vi","x-pseudo","yo","zh-cn","zh-hk","zh-mo","zh-tw"]

console.log('date-fns locales:')
process.stdout.write(JSON.stringify(date_fns_locales) + '\n');

console.log('momentjs locales: ')
process.stdout.write(JSON.stringify(momentLocales) + '\n');

const missingLocales = momentLocales.filter(o => !date_fns_locales.includes(o));

console.log('Missing locales: ')
process.stdout.write(JSON.stringify(missingLocales) + '\n');

console.log('Missing locales (full names): ')

let allCountries = {"BD": "Bangladesh", "BE": "Belgium", "BF": "Burkina Faso", "BG": "Bulgaria", "BA": "Bosnia and Herzegovina", "BB": "Barbados", "WF": "Wallis and Futuna", "BL": "Saint Barthelemy", "BM": "Bermuda", "BN": "Brunei", "BO": "Bolivia", "BH": "Bahrain", "BI": "Burundi", "BJ": "Benin", "BT": "Bhutan", "JM": "Jamaica", "BV": "Bouvet Island", "BW": "Botswana", "WS": "Samoa", "BQ": "Bonaire, Saint Eustatius and Saba ", "BR": "Brazil", "BS": "Bahamas", "JE": "Jersey", "BY": "Belarus", "BZ": "Belize", "RU": "Russia", "RW": "Rwanda", "RS": "Serbia", "TL": "East Timor", "RE": "Reunion", "TM": "Turkmenistan", "TJ": "Tajikistan", "RO": "Romania", "TK": "Tokelau", "GW": "Guinea-Bissau", "GU": "Guam", "GT": "Guatemala", "GS": "South Georgia and the South Sandwich Islands", "GR": "Greece", "GQ": "Equatorial Guinea", "GP": "Guadeloupe", "JP": "Japan", "GY": "Guyana", "GG": "Guernsey", "GF": "French Guiana", "GE": "Georgia", "GD": "Grenada", "GB": "United Kingdom", "GA": "Gabon", "SV": "El Salvador", "GN": "Guinea", "GM": "Gambia", "GL": "Greenland", "GI": "Gibraltar", "GH": "Ghana", "OM": "Oman", "TN": "Tunisia", "JO": "Jordan", "HR": "Croatia", "HT": "Haiti", "HU": "Hungary", "HK": "Hong Kong", "HN": "Honduras", "HM": "Heard Island and McDonald Islands", "VE": "Venezuela", "PR": "Puerto Rico", "PS": "Palestinian Territory", "PW": "Palau", "PT": "Portugal", "SJ": "Svalbard and Jan Mayen", "PY": "Paraguay", "IQ": "Iraq", "PA": "Panama", "PF": "French Polynesia", "PG": "Papua New Guinea", "PE": "Peru", "PK": "Pakistan", "PH": "Philippines", "PN": "Pitcairn", "PL": "Poland", "PM": "Saint Pierre and Miquelon", "ZM": "Zambia", "EH": "Western Sahara", "EE": "Estonia", "EG": "Egypt", "ZA": "South Africa", "EC": "Ecuador", "IT": "Italy", "VN": "Vietnam", "SB": "Solomon Islands", "ET": "Ethiopia", "SO": "Somalia", "ZW": "Zimbabwe", "SA": "Saudi Arabia", "ES": "Spain", "ER": "Eritrea", "ME": "Montenegro", "MD": "Moldova", "MG": "Madagascar", "MF": "Saint Martin", "MA": "Morocco", "MC": "Monaco", "UZ": "Uzbekistan", "MM": "Myanmar", "ML": "Mali", "MO": "Macao", "MN": "Mongolia", "MH": "Marshall Islands", "MK": "Macedonia", "MU": "Mauritius", "MT": "Malta", "MW": "Malawi", "MV": "Maldives", "MQ": "Martinique", "MP": "Northern Mariana Islands", "MS": "Montserrat", "MR": "Mauritania", "IM": "Isle of Man", "UG": "Uganda", "TZ": "Tanzania", "MY": "Malaysia", "MX": "Mexico", "IL": "Israel", "FR": "France", "IO": "British Indian Ocean Territory", "SH": "Saint Helena", "FI": "Finland", "FJ": "Fiji", "FK": "Falkland Islands", "FM": "Micronesia", "FO": "Faroe Islands", "NI": "Nicaragua", "NL": "Netherlands", "NO": "Norway", "NA": "Namibia", "VU": "Vanuatu", "NC": "New Caledonia", "NE": "Niger", "NF": "Norfolk Island", "NG": "Nigeria", "NZ": "New Zealand", "NP": "Nepal", "NR": "Nauru", "NU": "Niue", "CK": "Cook Islands", "XK": "Kosovo", "CI": "Ivory Coast", "CH": "Switzerland", "CO": "Colombia", "CN": "China", "CM": "Cameroon", "CL": "Chile", "CC": "Cocos Islands", "CA": "Canada", "CG": "Republic of the Congo", "CF": "Central African Republic", "CD": "Democratic Republic of the Congo", "CZ": "Czech Republic", "CY": "Cyprus", "CX": "Christmas Island", "CR": "Costa Rica", "CW": "Curacao", "CV": "Cape Verde", "CU": "Cuba", "SZ": "Swaziland", "SY": "Syria", "SX": "Sint Maarten", "KG": "Kyrgyzstan", "KE": "Kenya", "SS": "South Sudan", "SR": "Suriname", "KI": "Kiribati", "KH": "Cambodia", "KN": "Saint Kitts and Nevis", "KM": "Comoros", "ST": "Sao Tome and Principe", "SK": "Slovakia", "KR": "South Korea", "SI": "Slovenia", "KP": "North Korea", "KW": "Kuwait", "SN": "Senegal", "SM": "San Marino", "SL": "Sierra Leone", "SC": "Seychelles", "KZ": "Kazakhstan", "KY": "Cayman Islands", "SG": "Singapore", "SE": "Sweden", "SD": "Sudan", "DO": "Dominican Republic", "DM": "Dominica", "DJ": "Djibouti", "DK": "Denmark", "VG": "British Virgin Islands", "DE": "Germany", "YE": "Yemen", "DZ": "Algeria", "US": "United States", "UY": "Uruguay", "YT": "Mayotte", "UM": "United States Minor Outlying Islands", "LB": "Lebanon", "LC": "Saint Lucia", "LA": "Laos", "TV": "Tuvalu", "TW": "Taiwan", "TT": "Trinidad and Tobago", "TR": "Turkey", "LK": "Sri Lanka", "LI": "Liechtenstein", "LV": "Latvia", "TO": "Tonga", "LT": "Lithuania", "LU": "Luxembourg", "LR": "Liberia", "LS": "Lesotho", "TH": "Thailand", "TF": "French Southern Territories", "TG": "Togo", "TD": "Chad", "TC": "Turks and Caicos Islands", "LY": "Libya", "VA": "Vatican", "VC": "Saint Vincent and the Grenadines", "AE": "United Arab Emirates", "AD": "Andorra", "AG": "Antigua and Barbuda", "AF": "Afghanistan", "AI": "Anguilla", "VI": "U.S. Virgin Islands", "IS": "Iceland", "IR": "Iran", "AM": "Armenia", "AL": "Albania", "AO": "Angola", "AQ": "Antarctica", "AS": "American Samoa", "AR": "Argentina", "AU": "Australia", "AT": "Austria", "AW": "Aruba", "IN": "India", "AX": "Aland Islands", "AZ": "Azerbaijan", "IE": "Ireland", "ID": "Indonesia", "UA": "Ukraine", "QA": "Qatar", "MZ": "Mozambique"}
missingLocalesWithNames = missingLocales.map((code) => {
    const localeCodeArray = code.split('-')
    if (localeCodeArray.length == 1) {
        return ISO6391.getName(code)
    }
    return ISO6391.getName(localeCodeArray[0]) + '-' + allCountries[localeCodeArray[1].toUpperCase()]
})
process.stdout.write(JSON.stringify(missingLocalesWithNames) + '\n');

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 19, 2023

Quick update, using date-fns with dynamic loading is as easy as the following:

import { format, type Locale } from 'date-fns'
import { getLocale } from '@nextcloud/l10n'

const fetchLocale = async () => {
	try {
		return await import(`date-fns/locale/${getLocale()}/index.js`) as Locale
	} catch (error) {
		try {
			const code = getLocale().split('_')[0]
			console.debug(`Could not load locale '${getLocale()}', falling back to '${code}'`)
			return await import(`date-fns/locale/${code}/index.js`) as Locale
		} catch (error) {
			return undefined
		}
	}
}

let locale = undefined as Locale | undefined
fetchLocale().then(l => { locale = l })

export default {
	// ...
	methods: {
		lastModified(node: File|Node): string {
			const lastModified = node instanceof File
				? new Date(node.lastModified)
				: node.mtime
			if (lastModified) {
				return format(lastModified, 'PPP', { locale })
			}
			return t('Last modified date unknown')
		},
	}
}

We could easily have that tiny handler above in server so it's loaded only once and use setDefaultOptions({ locale })

@TeaDrinkingProgrammer
Copy link

TeaDrinkingProgrammer commented Oct 20, 2023

Looks good. What is the lastModified function for? I can't find it in the current implementation. Also, one caveat with the current code is mixed formats, like en_de. There have been some issues with that at Calender. TLDR: it is very important to involve Nextcloud Core so that users can only select locales that actually exist and not just any random combination (You can now select something like Mongolian with a Dutch locale, which makes no sense). An added advantage is that it might mean that people feel more motivated to create locales (I might make a en-nl locale myself).

Edit: Users can choose the language and locale independently, they cannot make random combinations in the locale but the locale and language can be different. EG: language Mongolian, locale Australian English). So it is important to make sure that the locales shown are based on the language. EG: If you choose English as a language, you can only choose Australian English, New Zealand English etc.

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 21, 2023

Looks good. What is the lastModified function for?

Just an example of what a real usage of format would look like 😉

(You can now select something like Mongolian with a Dutch locale, which makes no sense)

It makes no sense for YOU. But for me it might :)
I personally like having the French locale with the English language 🤷


Original discussion for Locale vs Language: nextcloud/server#15457

@TeaDrinkingProgrammer
Copy link

Oh no, I didn't mean to say that something like that shouldn't be allowed, all I'm saying is we can't gaurantee it. What should something like english and mongolian do? We don't have that locale. In fact, we have quite few locales. I would love to support all of them, truly, but now it looks like they are supported when they are in fact not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants