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

Required parameters in constructors vs. property bag in from() #340

Closed
ptomato opened this issue Feb 6, 2020 · 3 comments · Fixed by #346
Closed

Required parameters in constructors vs. property bag in from() #340

ptomato opened this issue Feb 6, 2020 · 3 comments · Fixed by #346
Assignees

Comments

@ptomato
Copy link
Collaborator

ptomato commented Feb 6, 2020

In new Temporal.DateTime(y, mon, d, h, min, s, ms, µs, ns) the first five parameters are required. Whereas, Temporal.DateTime.from(propertyBag) is quite different: In the spec, all the parameters are required (see the ToDateTime operation.) In the polyfill, none are required although at least one must be present (from({}) doesn't work.)

All of this seems inconsistent to me. Taking account of the discussion in #41, why would half the constructor parameters be optional and half required, when there are (IMO) reasonable default values for all of them? And I'd argue that if you can't call new Temporal.DateTime(2000) because there's no reasonable default value, then you shouldn't be able to call Temporal.DateTime.from({year: 2000}), and conversely if you can do the second one then you should be able to do the first one.

I'd propose making all the parameters optional everywhere. If you can call Temporal.DateTime.from({month: 5}) and get midnight, May 1, 1BCE then why not {} for midnight, January 1, 1BCE? And it would allow doing stuff like Temporal.DateTime.from(monthDayObject).with({year: 2000}) to get the the 2000 occurrence of the calendar month from your Temporal.MonthDay object.

@sffc
Copy link
Collaborator

sffc commented Feb 13, 2020

From 2020-02-19 Temporal call: year, month, and day should be required; all others should be optional. Add calendar as the last argument.

@sffc
Copy link
Collaborator

sffc commented Feb 14, 2020

One question: are year, month, and day required in both the constructor and .from(), or only in the constructor? I'd say we should require them to all be present in both places. If you want a year/month or a month/day, we have types for that.

@ptomato
Copy link
Collaborator Author

ptomato commented Feb 14, 2020

In the PR attached here, they are all required in both places.

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 a pull request may close this issue.

2 participants