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

[feature] Support add(n, 'isoweek') and subtract(n, 'isoweek') #4518

Closed
thomashusa opened this issue Mar 28, 2018 · 2 comments
Closed

[feature] Support add(n, 'isoweek') and subtract(n, 'isoweek') #4518

thomashusa opened this issue Mar 28, 2018 · 2 comments

Comments

@thomashusa
Copy link

thomashusa commented Mar 28, 2018

Description of the Issue and Steps to Reproduce:
Currently adding or subtracting 'isoweek' from a moment returns the same moment:

> m = moment()
moment("2018-03-28T13:12:35.901")
> m.add(1, 'isoweek')
moment("2018-03-28T13:12:35.901")

It would be convenient if isoweek was aliased to week in this use case so that the following would be returned:

> m = moment()
moment("2018-03-28T13:12:35.901")
> m.add(1, 'isoweek')
moment("2018-04-04T13:12:35.901")

The reason behind the request is that frequently have to "chunk" time series data by a user provided period (such as a week). moment.js makes it very simple to find the start of a chunk by doing something like moment(timestamp).startOf(period). However, if the period is isoweek, I can not simply calculate the next chunk as usual via moment.add(1, period) since this will return the same timestamp and not a timestamp 1 week in the future.

Environment:
Node.JS 8.9.4 on Ubuntu 16.0.4

@icambron
Copy link
Member

icambron commented Apr 3, 2018

I think this is a reasonable request, since we'd just make it do the same thing as week in add and subtract.

@adamreisnz
Copy link

I want to +1 this, but at the same time I'm really annoyed that moment chose to start weeks on a sunday by default and not simply use the iso default. That would make everyone's lives a lot easier:

const period = 'week';
moment().startOf(period).subtract(1, period);

Oops doesn't work as intended, I get sunday instead of monday.

const period = 'isoWeek';
moment().startOf(period).subtract(1, period);

Oops doesn't work as intended, moment doesn't know how to handle subtraction of an iso week 🤦‍♂️

@ichernev ichernev changed the title Support add(n, 'isoweek') and subtract(n, 'isoweek') [feature] Support add(n, 'isoweek') and subtract(n, 'isoweek') Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants