nextcloud-vue
nextcloud-vue copied to clipboard
Auto load locale from nextcloud for the Datepicker
Fix #137
Will moment
then be included twice if the app requires it as well? Or even be loaded three times by the server as well?
@raimund-schluessler I don't get your question :thinking:
The server ships moment.js
in version 2.18.1 https://github.com/nextcloud/server/blob/master/package.json#L41
E.g. the contacts app requests moment.js
in version 2.23.0 https://github.com/nextcloud/contacts/blob/master/package.json#L42
With this PR nextcloud-vue requires moment.js
in version 2.22.2.
So my question is, will three different versions of moment.js
be loaded when you open the contacts app? One from the server, and two with the contacts.js
package (one directly, one in nextcloud-vue)?
Each moment.js
version is at least 65 kB (gzip), more likely 225 kB (see https://bundlephobia.com/[email protected]). This sounds like quite some overhead.
No, on contacts we use moment for the datepicker as well, so once this is merged, we will remove it from contacts :) Though this is a good question, how should we do that. How can we make sure we don't load it multipel times. For calendar (for example) they'll probably have to also load it for the whole app. If we have the same version as nextcloud-vue, can webpack merge the two of those?
On another note, with dynamic loading, we can load the locale separately from moment core, so it will be moment (48.9KB) + locale (avg 1.5KB). See #13
Maybe this might help https://github.com/webpack/docs/wiki/optimization#deduplication
Maybe this might help https://github.com/webpack/docs/wiki/optimization#deduplication
This option is redundant in webpack 2.
Seems perfect then!! Thanks @raimund-schluessler :hugs:
Seems perfect then!! Thanks @raimund-schluessler hugs
This option is redundant in webpack 2.
well :man_shrugging:
My 2c: for independent, isolated code like the Vue components or an app bundle it should be possible add async loading of locales for moment. But in Server we just assume that they are ready on page load. I don't think we can fix that.
For that webpack deduplication to work I assume the versions have to be identical. Webpack must not deduplicate otherwise.
But in Server we just assume that they are ready on page load. I don't think we can fix that.
What do you mean by that? Couldn't we not load moment in our apps and use the server's version?
Couldn't we not load moment in our apps and use the server's version?
We can. Tasks does not load its own moment version, just uses the server one.
Sure you can use the server version, but this makes you dependent on server. And in the opposite direction updates of moment in server are more error prone as apps rely on it as well. This is the same issue with jQuery and the other dependencies.
Neither using the server version nor shipping a duplication with every app is ideal. But the latter at least offers isolation and easier maintenance.
So the best would be to have moment integrated with nextcloud-vue, then?
Yes. Deduplication from server is more important than the few kb. Also note that this PR is a bit old. So in the future contacts and the vue-components will probably ship the same moment version so it will be merged.
Any work on this? Date picker's localisation would be useful.
@dartcafe none so far. We should try to see what we can really do on this ! :thinking:
@ChristophWurst could we have locale and such in nextcloud-server for this one?
@ChristophWurst could we have locale and such in nextcloud-server for this one?
http://blog.wuc.me/nextcloud-server/modules/l10n.html#getlocale?
No, sorry, I meant window.monthNamesShort, window.dayNamesShort & window.firstDay
What's the status here? :)
What's the status here? :)
no idea, @ChristophWurst would have liked a better way of loading the locales from server, but there is still no packages like @nextcloud/lang or something!
I guess we should maybe get this in? What do you think @ChristophWurst ?
Also, it means we will now ship moment and will require the app that uses our lib to implement the dynamic loading themselves
no idea, @ChristophWurst would have liked a better way of loading the locales from server, but there is still no packages like @nextcloud/lang or something!
Well, there is @nextcloud/l10n
, which provides getLocale()
and getLanguage()
Well, there is
@nextcloud/l10n
, which providesgetLocale()
andgetLanguage()
My bad, it was about window.firstDay, window.monthNamesShort and window.dayNamesShort :)
My bad, it was about window.firstDay, window.monthNamesShort and window.dayNamesShort
I guess we could include a getFirstDay(), getMonthNamesShort(), etc. in @nextcloud/l10n
?
@ChristophWurst
https://github.com/nextcloud/nextcloud-l10n/pull/30
nextcloud/nextcloud-l10n#30 got merged. So we now have:
-
getFirstDay()
-
getDayNames()
-
getDayNamesShort()
-
getDayNamesMin()
-
getMonthNames()
-
getMonthNamesShort()
@georgehrke mind to take over this one? :)
Should we include the async loading of locales here or rather right away in @Nextcloud/moment
?
@georgehrke even if you include it here, you will need your own implementatin of the dynamic moment loading on your webpack config :)
@georgehrke even if you include it here, you will need your own implementatin of the dynamic moment loading on your webpack config :)
Because of https://github.com/nextcloud/nextcloud-vue/pull/146#discussion_r333006574?
Closing since https://github.com/nextcloud/nextcloud-vue/pull/2397 replaced it.