Display format update

[schiem]

This adds many of the missing display formats referenced in #60 . It does add an additional ~30 lines of code, which is ~10% of the entire library size. I think that some of the options are worthwhile, but there are some that might not be needed.

[codecov-io]

Codecov Report

Merging #68 into master will decrease coverage by 1.08%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
- Coverage     100%   98.91%   -1.09%     
==========================================
  Files           3        3              
  Lines         175      185      +10     
  Branches       27       33       +6     
==========================================
+ Hits          175      183       +8     
- Misses          0        2       +2

Impacted Files	Coverage Δ
src/index.js	98.67% <81.81%> (-1.33%)	⬇️

---

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 0bcf96b...dac9b7e. Read the comment docs.

[iamkun]

Thanks. That's quite a lot of code for this tiny project. We may need a discussion about the display format tokens that we really need. PR pending

[schiem]

I definitely agree. I broke all of them out into separate commits so it would be possible to remove any of the formats that aren't needed.

[dragonfire1119]

The AM & PM would be nice to have. Why I couldn't even switch my projects over to using DayJS is because of the formatting options. I completely understand you want to keep it small though.

[schiem]

Would there possibly be a way to extend the format function so that it could accept new display format options from the user, so users could include any extra format options they need without increasing the library size? Several users have mentioned specific display format options that they would use that are missing, but I've also seen several users say that they don't think any more are required since they don't personally use them.

That leads me to believe that the solution that would make the most users happy would be finding a way to extend it so users who need the extra format option could have them by including them separately, but the library would stay compact for users who don't need them.

Barring that, I think that at most 1-2 of the newly introduced formats should be merged in, for the sake of the size of the library.

[iamkun]

Think we might need a plugin system, like jQuery.

import dayjs from ''dayjs'
import { advancedFormat } from 'dayjs/plugin'
dayjs.extend(advancedFormat) // will overwrite core.format

//then

dayjs.format('full format options support')

[warapitiya]

I completely agree with the library size, and the plugin system is a great idea. Anyway, we needed this sort of a formatting technics. I would love to see this get implemented soon.

[schiem]

I started working on a plugin system.

It works by taking in objects in the format of:

{
  name : "methodName",
  method : function() { //plugin method }
}

So to override the format method, you would do something like:

{
  name : "format",
  method : ...
}

The plugins are then included in the core by doing:

import advancedFormat from "./plugins/advancedFormat"

dayjs().extend(advancedFormat);
dayjs().format(); //New format

This operates on the class prototype, so any calls to the function after the plugin is loaded will use the new function.

I also reduced the additional formats in the core down to h/hh and a/A for 12 hour time and AM/PM. The rest of the new formats are included in the plugin. The downside to this plugin approach is that all of the code for the format function had to be copied into the advancedFormat plugin as well, but I doubt that the core format function will be extended too much in the future.

Let me know what you think!

[iamkun]

Looks cool.

But seems too many repeated code in this plugin system design (utils, old format func).
What I am thinking is:
1 assign core Utils to this.$u so that we can use it directly instead of imported it

this.$u = Utils  

2 while extend an API like format, we might could pass the argument to the core.format then pass to plugin.format.

dayjs.extend(function(args) {
var oldFormat = this.format
var oldresult = this.format(args) // something like this "2018-01-01 Do" 'Do' is not supported in core.format
var result = pluginFormat(oldresult) // process 'Do' return "2018-01-01 1st"
return result
})

3 global plugin
I don't have any idea yet. But could we extend our plugin globally ? Like

dayjs.extend(advFromat) // instead of `dayjs().extend(advFromat)`

[schiem]

I definitely agree with 1, I'll move the Utils to a member object so it will be accessible.

I really like the approach in 2, since it reduced the amount of duplicated code. Since the current approach involves overwriting the method, I don't think directly calling the old method will work (but I'll need to test it). For example:

function(args) {
  var oldFormat = this.format //format has already been overwritten, so this.format is the current function
  ... 
}

Instead, could we move the old functions into an old member object? Like so:

extend(plugin) {
  this.old[plugin.name] = this[plugin.name]
  this[plugin.name] = plugin.method
}

And then in the plugin:

function(args) {
  var oldFormat = this.old['format']
  var oldResult = oldFormat(args)
  ...
}

For number 3, could you explain what you mean a bit more? Even though the extend method is being called on an instance, it's overwriting the original prototype, which is about as global as it gets (it will retro-actively replace the method in any existing instances).
Here's some code I just ran to demonstrate:

var dayjs = require("./dist/dayjs.min.js");

var dayjsInstance = dayjs();
console.log(dayjsInstance.format()); //Output: 2018-05-01T13:31:07-04:00

dayjs().extend({
    name : "format", 
    method : function() { 
        return "firing new format"; 
    }
}); 

console.log(dayjsInstance.format()); //Output: firing new format
console.log(dayjs().format()); //Output: firing new format

That might not be what a user would expect to happen though, since it is called on an instance instead of the base model.

[iamkun]

@schiem Would you mind cherry-pick your commits about h/hh/a/A as well as related test file into a separate pr please? So that I could merge it before we finish our plugin system design.

[iamkun]

For number 3, my mistake sorry. I haven't noticed that you've overtired it's prototype. I thought it was an instance level change not global.
Still, our user may have the same confusion like me. So how can we make this api more friendly? May be like this dayjs.extend(advFromat)

[schiem]

@iamkun Absolutely, I'll move those commits over when I get home from work.

One of the issues with using the dayjs object instead of dayjs() is that dayjs is just a function that returns a new instance, like so:

var dayjs = function(config) {
    return new Dayjs(config);
}

which means that the dayjs variable doesn't have access to the underlying Dayjs class. When I get home I'll play around with doing something like:

let dayjs = (config) => {
    return new Dayjs(config);
}
dayjs.extend = (plugin) => {
    Dayjs[plugin.name] = plugin.method;
}
export default dayjs(config);

But I'll have to dig around with that before I can say for sure that that approach will work.

[schiem]

@iamkun I've split that into 2 new separate PRs (#91 and #92), so I'll close this one out. #92 cover all three points you brought up earlier.