add setWeek fucntion to weekOfYear #368

aquaminer · 2018-10-06T08:06:40Z

No description provided.

codecov-io · 2018-10-08T18:42:32Z

Codecov Report

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

@@          Coverage Diff          @@
##           master   #368   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          56     56           
  Lines         491    495    +4     
  Branches       76     79    +3     
=====================================
+ Hits          491    495    +4

Impacted Files	Coverage Δ
src/plugin/weekOfYear/index.js	`100% <100%> (ø)`	⬆️

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 bdfb7c8...79ac1bc. Read the comment docs.

naulacambra · 2018-10-09T08:08:07Z

You should add tests so the code coverage remains at 100%.

aquaminer · 2018-10-10T19:21:58Z

@naulacambra added expect(dayjs().setWeek(week)).toBe(moment().week(week)) and received this

full:

 FAIL  test/plugin/weekOfYear.test.js
  ● Week of year

    expect(received).toBe(expected) // Object.is equality
    
    Expected value to be:
      "2018-06-06T19:17:10.673Z"
    Received:
      "2018-06-06T19:17:10.673Z"
    
    Difference:
    
    Compared values serialize to the same structure.
    Printing internal object structure without calling `toJSON` instead.
    
    - Expected
    + Received
    
    - Moment {
    -   "_d": 2018-06-06T19:17:10.673Z,
    -   "_isAMomentObject": true,
    -   "_isUTC": false,
    -   "_isValid": true,
    -   "_locale": Locale {
    -     "_abbr": "en",
    -     "_calendar": Object {
    -       "lastDay": "[Yesterday at] LT",
    -       "lastWeek": "[Last] dddd [at] LT",
    -       "nextDay": "[Tomorrow at] LT",
    -       "nextWeek": "dddd [at] LT",
    -       "sameDay": "[Today at] LT",
    -       "sameElse": "L",
    -     },
    -     "_config": Object {
    -       "abbr": "en",
    -       "calendar": Object {
    -         "lastDay": "[Yesterday at] LT",
    -         "lastWeek": "[Last] dddd [at] LT",
    -         "nextDay": "[Tomorrow at] LT",
    -         "nextWeek": "dddd [at] LT",
    -         "sameDay": "[Today at] LT",
    -         "sameElse": "L",
    -       },
    -       "dayOfMonthOrdinalParse": /\d{1,2}(th|st|nd|rd)/,
    -       "invalidDate": "Invalid date",
    -       "longDateFormat": Object {
    -         "L": "MM/DD/YYYY",
    -         "LL": "MMMM D, YYYY",
    -         "LLL": "MMMM D, YYYY h:mm A",
    -         "LLLL": "dddd, MMMM D, YYYY h:mm A",
    -         "LT": "h:mm A",
    -         "LTS": "h:mm:ss A",
    -       },
    -       "meridiemParse": /[ap]\.?m?\.?/i,
    -       "months": Array [
    -         "January",
    -         "February",
    -         "March",
    -         "April",
    -         "May",
    -         "June",
    -         "July",
    -         "August",
    -         "September",
    -         "October",
    -         "November",
    -         "December",
    -       ],
    -       "monthsShort": Array [
    -         "Jan",
    -         "Feb",
    -         "Mar",
    -         "Apr",
    -         "May",
    -         "Jun",
    -         "Jul",
    -         "Aug",
    -         "Sep",
    -         "Oct",
    -         "Nov",
    -         "Dec",
    -       ],
    -       "ordinal": [Function ordinal],
    -       "relativeTime": Object {
    -         "M": "a month",
    -         "MM": "%d months",
    -         "d": "a day",
    -         "dd": "%d days",
    -         "future": "in %s",
    -         "h": "an hour",
    -         "hh": "%d hours",
    -         "m": "a minute",
    -         "mm": "%d minutes",
    -         "past": "%s ago",
    -         "s": "a few seconds",
    -         "ss": "%d seconds",
    -         "y": "a year",
    -         "yy": "%d years",
    -       },
    -       "week": Object {
    -         "dow": 0,
    -         "doy": 6,
    -       },
    -       "weekdays": Array [
    -         "Sunday",
    -         "Monday",
    -         "Tuesday",
    -         "Wednesday",
    -         "Thursday",
    -         "Friday",
    -         "Saturday",
    -       ],
    -       "weekdaysMin": Array [
    -         "Su",
    -         "Mo",
    -         "Tu",
    -         "We",
    -         "Th",
    -         "Fr",
    -         "Sa",
    -       ],
    -       "weekdaysShort": Array [
    -         "Sun",
    -         "Mon",
    -         "Tue",
    -         "Wed",
    -         "Thu",
    -         "Fri",
    -         "Sat",
    -       ],
    -     },
    -     "_dayOfMonthOrdinalParse": /\d{1,2}(th|st|nd|rd)/,
    -     "_dayOfMonthOrdinalParseLenient": /\d{1,2}(th|st|nd|rd)|\d{1,2}/,
    -     "_invalidDate": "Invalid date",
    -     "_longDateFormat": Object {
    -       "L": "MM/DD/YYYY",
    -       "LL": "MMMM D, YYYY",
    -       "LLL": "MMMM D, YYYY h:mm A",
    -       "LLLL": "dddd, MMMM D, YYYY h:mm A",
    -       "LT": "h:mm A",
    -       "LTS": "h:mm:ss A",
    -     },
    -     "_meridiemParse": /[ap]\.?m?\.?/i,
    -     "_months": Array [
    -       "January",
    -       "February",
    -       "March",
    -       "April",
    -       "May",
    -       "June",
    -       "July",
    -       "August",
    -       "September",
    -       "October",
    -       "November",
    -       "December",
    -     ],
    -     "_monthsShort": Array [
    -       "Jan",
    -       "Feb",
    -       "Mar",
    -       "Apr",
    -       "May",
    -       "Jun",
    -       "Jul",
    -       "Aug",
    -       "Sep",
    -       "Oct",
    -       "Nov",
    -       "Dec",
    -     ],
    -     "_relativeTime": Object {
    -       "M": "a month",
    -       "MM": "%d months",
    -       "d": "a day",
    -       "dd": "%d days",
    -       "future": "in %s",
    -       "h": "an hour",
    -       "hh": "%d hours",
    -       "m": "a minute",
    -       "mm": "%d minutes",
    -       "past": "%s ago",
    -       "s": "a few seconds",
    -       "ss": "%d seconds",
    -       "y": "a year",
    -       "yy": "%d years",
    -     },
    -     "_week": Object {
    -       "dow": 0,
    -       "doy": 6,
    -     },
    -     "_weekdays": Array [
    -       "Sunday",
    -       "Monday",
    -       "Tuesday",
    -       "Wednesday",
    -       "Thursday",
    -       "Friday",
    -       "Saturday",
    -     ],
    -     "_weekdaysMin": Array [
    -       "Su",
    -       "Mo",
    -       "Tu",
    -       "We",
    -       "Th",
    -       "Fr",
    -       "Sa",
    -     ],
    -     "_weekdaysShort": Array [
    -       "Sun",
    -       "Mon",
    -       "Tue",
    -       "Wed",
    -       "Thu",
    -       "Fri",
    -       "Sat",
    -     ],
    -     "ordinal": [Function ordinal],
    -   },
    -   "_pf": Object {
    -     "charsLeftOver": 0,
    -     "empty": false,
    -     "invalidFormat": false,
    -     "invalidMonth": null,
    -     "iso": false,
    -     "meridiem": null,
    -     "nullInput": false,
    -     "overflow": -2,
    -     "parsedDateParts": Array [],
    -     "rfc2822": false,
    -     "unusedInput": Array [],
    -     "unusedTokens": Array [],
    -     "userInvalidated": false,
    -     "weekdayMismatch": false,
    -   },
    + Dayjs {
    +   "$D": 6,
    +   "$H": 22,
    +   "$L": "en",
    +   "$M": 5,
    +   "$W": 3,
    +   "$d": 2018-06-06T19:17:10.673Z,
    +   "$m": 17,
    +   "$ms": 673,
    +   "$s": 10,
    +   "$y": 2018,
      }

      20 |   expect(dayjs().week()).toBe(moment().week())
      21 | 
    > 22 |   expect(dayjs().setWeek(week)).toBe(moment().week(week))
      23 | })
      24 | 
      
      at Object.<anonymous> (test/plugin/weekOfYear.test.js:22:33)

please tell me how to fix?

naulacambra · 2018-10-11T07:51:52Z

You're comparing a dayjs instance, with a moment instance, so they will never be the same.

The first test, works because the function .week() return an integer representing to which week of the year belongs the current date. Your new method, (I assume) sets the week, changing the current date, and returns the instance itself.

expect(dayjs().week()).toBe(moment().week()) // expect([int]).toBe([int]) - apples to apples

expect(dayjs().setWeek(week)).toBe(moment().week(week)) // expect([dayjs]).toBe([moment]) - apples to oranges

You should change the test to

expect(dayjs().setWeek(week).week()).toBe(moment().week(week).week()) // expect([int]).toBe([int]) - apples to apples

And also, add another test like

expect(dayjs().setWeek(week).date()).toBe(moment().week(week).date()) // expect([int]).toBe([int]) - apples to apples

I hope it helps

@naulacambra

thanks @naulacambra

naulacambra · 2019-01-17T15:29:23Z

BTW, reviewing again this PR, I noticed that you are adding a new method, setWeek, instead of adding an optional parameter to the method week, just like moment does.

moment().week(Number);
moment().week(); // Number

Since this lib bases its API on moment's api, could you change that?

And by other hand, could you add an alias, weeks? That would be awesome 😀

moment().week(Number);
moment().week(); // Number
moment().weeks(Number);
moment().weeks(); // Number

MomentJS Docs

naulacambra · 2019-01-18T08:49:42Z

src/plugin/weekOfYear/index.js

@@ -2,7 +2,12 @@ import { MS, Y, D, W } from '../../constant'

 export default (o, c, d) => {
  const proto = c.prototype
-  proto.week = function () {
+  proto.week = function (week = 0) {
+    if (week > 0) {


Instead of checking if week is positive, you should check if it's undefined.

Momentjs allows set a negative week

console.log(moment('2018-01-20').week(-1).format()); // "2017-12-23T00:00:00+01:00"

So I think that we should as well

src/plugin/weekOfYear/index.js

naulacambra · 2019-01-21T08:38:34Z

src/plugin/weekOfYear/index.js

@@ -12,4 +15,7 @@ export default (o, c, d) => {
    const diffInWeek = this.diff(compareDay, W, true)
    return Math.ceil(diffInWeek)
  }
+  proto.weeks = function (week = 0) {


Be aware that, if user call weeks alias, without arguments, the default value week = 0 will set the week to 0.

dayjs('2018-01-01').week() // 1 dayjs('2018-01-01').weeks() // 0

You should change the default value to null, the same for week()

proto.weeks = function (week = null) { return this.week(week) }

aquaminer · 2019-01-21T10:03:38Z

@naulacambra sorry, did not pay attention. Fixed

iamkun · 2019-02-05T14:29:38Z

🎉 This PR is included in version 1.8.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

aquaminer added 2 commits October 6, 2018 11:06

add setWeek

8b7f26a

compliance with eslint rules

f7cbee3

aquaminer added 2 commits October 11, 2018 20:53

add setWeek test

9230ac8

thanks @naulacambra

compliance with eslint rules

57882b2

aquaminer added 3 commits January 17, 2019 20:44

Merge branch 'master' into patch-2

3f7e60e

remove setWeek method, set week validation and add weeks alias

86afa45

😐 fix

4da82e5

naulacambra reviewed Jan 18, 2019

View reviewed changes

aquaminer added 2 commits January 20, 2019 11:34

test: allow setting negative week

3c00791

rm week validate

b155101

naulacambra suggested changes Jan 21, 2019

View reviewed changes

fix weeks func

70411b6

naulacambra approved these changes Jan 21, 2019

View reviewed changes

Merge branch 'master' into patch-2

79ac1bc

iamkun merged commit 142b763 into iamkun:master Feb 5, 2019

iamkun added the released label Feb 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add setWeek fucntion to weekOfYear #368

add setWeek fucntion to weekOfYear #368

aquaminer commented Oct 6, 2018

codecov-io commented Oct 8, 2018 •

edited

Loading

naulacambra commented Oct 9, 2018

aquaminer commented Oct 10, 2018 •

edited

Loading

naulacambra commented Oct 11, 2018

naulacambra commented Jan 17, 2019

naulacambra Jan 18, 2019

naulacambra Jan 21, 2019 •

edited

Loading

aquaminer commented Jan 21, 2019

iamkun commented Feb 5, 2019

add setWeek fucntion to weekOfYear #368

add setWeek fucntion to weekOfYear #368

Conversation

aquaminer commented Oct 6, 2018

codecov-io commented Oct 8, 2018 • edited Loading

Codecov Report

naulacambra commented Oct 9, 2018

aquaminer commented Oct 10, 2018 • edited Loading

naulacambra commented Oct 11, 2018

naulacambra commented Jan 17, 2019

naulacambra Jan 18, 2019

Choose a reason for hiding this comment

naulacambra Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

aquaminer commented Jan 21, 2019

iamkun commented Feb 5, 2019

codecov-io commented Oct 8, 2018 •

edited

Loading

aquaminer commented Oct 10, 2018 •

edited

Loading

naulacambra Jan 21, 2019 •

edited

Loading