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

Introduce an UTC mode simpler than #168 #326

Closed
wants to merge 3 commits into from

Conversation

prantlf
Copy link
Contributor

@prantlf prantlf commented Sep 5, 2018

The "UTC mode" means, that he constructor assumes the UTC date in the input string and the instance methods return and accept date parts (year, month etc.) in UTC.

Additions

  • Constructor accepts a boolean flag to enable the UTC mode: { utc: true }.
  • Method utc() returns a clone of the instance in the UTC mode.
  • Method local() returns a clone of the instance in the non-UTC (local) mode.
  • Method isUTC() checks, if the instance is in the UTC mode.
  • Method utcOffset() returns the time zone offset to UTC consistently with Date.prototype.getTimezoneOffset()

Differences to the "normal" (local) dayjs mode

  • Assume UTC as the time zone of the string passed to the constructor - for example, "2018-10-28" will be parsed as "2018-10-28T00:00:00Z". But only if the time zone is omitted. If you end the string tith a TZ offset - "2018-10-28T00:00:00+02:00", it will be retained and the whole date will be converted to UTC, when iniitalizing the datejs object.
  • Methods returning the date parts like year(), month() etc. return UTC parts using Date.propertotype.getUTC*() methods.
  • Methods manipulating the date - set(), add() etc. - work with the UTC values of the date parts, as the getters mentioned above.
  • The format() method uses the UTC getters too and always formats the time zone as "+00:00".
  • The utcOffset() method always returns zero.

If a Day.js instance is constructed without the time part, it will appear, as if the instance runs in a "date-only" mode, where no conversion from UTC to the local time takes place, like the Date instance would do it. It can be used for attributes, where the time does not make sense and only the date should be maintained. If only the date applies, it should not be affected by time zone changes.

@codecov-io
Copy link

codecov-io commented Sep 5, 2018

Codecov Report

Merging #326 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #326   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     39    +7     
  Lines         386    445   +59     
  Branches       53     69   +16     
=====================================
+ Hits          386    445   +59
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️
src/locale/sr-cyrl.js 100% <0%> (ø)
src/locale/nn.js 100% <0%> (ø)
src/locale/sr.js 100% <0%> (ø)
src/locale/pl.js 100% <0%> (ø)
src/locale/sk.js 100% <0%> (ø)
src/locale/hu.js 100% <0%> (ø)
src/locale/fi.js 100% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b14bdd1...e640b57. Read the comment docs.

@prantlf
Copy link
Contributor Author

prantlf commented Sep 5, 2018

I am considering a plugin with this implementation, but I am not sure, if it fits such pervasive implementation. The, wrapper and parseDate would need to be overridden. startOf and $set would be a copy & paste for a bigger part, and index.d.ts is not extensible.

@prantlf prantlf mentioned this pull request Sep 5, 2018
2 tasks
Additions:

* Constructor accepts a boolean flag to enable the UTC mode: { utc: true }.
* Method utc() returns a clone of the instance in the UTC mode.
* Method local() returns a clone of the instance in the non-UTC (local) mode.
* Method isUTC() checks, if the instance is in the UITC mode.
* Method utcOffset() returns the time zone offset to UTC consistently with Date.prototype.getTimezoneOffset()

Differences to the "normal" (local) dayjs mode:

* Assume UTC as the time zone of the string passed to the constructor - for example, "2018-10-28" will be parsed as "2018-10-28T00:00:00Z". *But only if the time zone is omitted.* If you end the string tith a TZ offset - "2018-10-28T00:00:00+02:00", it will be retained and the whole date will be converted to UTC, when iniitalizing the datejs object.
* Methods returning the date parts like year(), month() etc. return UTC parts using Date.propertotype.getUTC* methods.
* Methods manipulating the date - set() - work with the UTC values of the date parts, as the getters above.
* The format() method always formats the time zone as "+00:00".
* The utcOffset() method always returns zero.
@prantlf
Copy link
Contributor Author

prantlf commented Sep 9, 2018

I wonder if you liked more the interface:

dayjs.utc('2018-09-25')

instead of the current:

dayjs('2018-09-25', { utc: true })

I introduced the latter, because static methods may feel like having a global effect on dayjs, like locale.

@prantlf
Copy link
Contributor Author

prantlf commented Sep 10, 2018

I prototyped an implementation of the UTC mode as a plugin the branch https://github.com/prantlf/dayjs/commits/utc-mode-plugin. It is rather an example, what should not be a plugin :-)

Leaving Day.js intact is impossible, because functions like wrapper are not accessed vial Utils.wrapper and thus not overriddable. I made a minimum change to Day.js to be able to write the plugin in the first commit. The first version of the plugin in the second commit was a brutal copy & paste, especially parseDate, startOf and $set.

Plugins are supposed to extend Day.js with additional functionality. Not to copy & paste Day.js logic to be able to add a feature to it. It would tightly couple the plugin to a particular Day.js version. Any fixes and enhancements done in Day.js would have to be copied to the plugin too. If such extensibility is desired, an interface in Day.js should be introduced to allow it.

I tried to put the differences between the local time zone and UTC modes on the interface. Basically, it is the Date construction and date part setters. I ended up with a couple of methods, which no other plugin will use, than the UTC mode support. I added two other commits for the date creation and the setters, but such interface feels weird. Moreover, using utilities like wrapper always with the source object (Utils.wrapper) will cause mistakes, because it is not usual., Especially in times, when JavaScript developers embrace object destructuring.

UTC mode does not feel such an exotic feature, or to increase the size so much, to leave it out of the core library.

@sahibjotsaggu
Copy link

Any update on when this will be merged and released?

@prantlf
Copy link
Contributor Author

prantlf commented Sep 27, 2018

Eventually , I added the static method utc as an alternative way to create a UTC instance.

dayjs.utc('2018-09-25')

It fits the purpose of Day.js to help migration from Moment.js. And the code is shorter, that the constructor option.

Repository owner deleted a comment from prantlf Feb 24, 2019
Repository owner deleted a comment from sahibjotsaggu Feb 24, 2019
Repository owner deleted a comment from prantlf Feb 24, 2019
@iamkun
Copy link
Owner

iamkun commented Mar 6, 2019

I think put in a plugin is better, #517.

Referenced some logic and test suits from this PR, great thanks.

@iamkun iamkun closed this Mar 6, 2019
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.

4 participants