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

Add format object type to index.d.ts #1572

Merged
merged 1 commit into from
Jul 12, 2021
Merged

Add format object type to index.d.ts #1572

merged 1 commit into from
Jul 12, 2021

Conversation

always-maap
Copy link
Contributor

@always-maap always-maap commented Jul 9, 2021

Hi. Dayjs is a super flexible library. I love working with it, writing plugins, and contributing to it. We can change anything with plugins but the typing part does not come along well enough. Using interface and not using type aliases will improve typing for plugin authors so much better. I changed one of the more important ones in this PR.

dayjs(new Date(), { multipleLocales: [] })

or any other example. the point is more flexibility in typing for plugin authors.
if there is a way to solve this problem currently or any planning for changing the whole typing system(v2 doesn't hit that), let me know <333

@codecov
Copy link

codecov bot commented Jul 9, 2021

Codecov Report

Merging #1572 (343db5e) into dev (08adda5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1572   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          177       177           
  Lines         1989      1989           
  Branches       505       505           
=========================================
  Hits          1989      1989           

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 08adda5...343db5e. Read the comment docs.

@iamkun
Copy link
Owner

iamkun commented Jul 9, 2021

actually, dayjs(data, config) is not a recommend API in our docs. It increases the complexity of the constructor to our end user.

@always-maap
Copy link
Contributor Author

always-maap commented Jul 9, 2021

actually, dayjs(data, config) is not a recommend API in our docs. It increases the complexity of the constructor to our end user.

By not being recommended you mean even the third-party library that has their own small api(I mean will never be part of official plugins)?
I think using those two concepts in declaration files is still a good idea. Also, I'm not using it for all configs. only configs that effects output directly aka formating configs. e.g more properties in output. Totally agree with you on this page. sorry for kinda broken en <333

@addisonElliott
Copy link

actually, dayjs(data, config) is not a recommend API in our docs. It increases the complexity of the constructor to our end user.

If I understand you correctly, you're saying the recommended approach by Day.js is to have something like dayjs(data).tz(xxx) over dayjs(data, { tz: xxx }. I see where you're coming from.

However, I don't think this means we should restrict 3rd party plugins to using this approach. If someone has a 3rd party plugin, I think it's up to them if they want to add an attribute to the config object or create a separate function to set the attribute. Plus, adding an attribute to the config object is already possible, rather the issue is the Typescript definitions and being able to augment the interface.

So, given that, I think this PR is beneficial to people working on customizing Day.js 👍

@always-maap
Copy link
Contributor Author

actually, dayjs(data, config) is not a recommend API in our docs. It increases the complexity of the constructor to our end user.

If I understand you correctly, you're saying the recommended approach by Day.js is to have something like dayjs(data).tz(xxx) over dayjs(data, { tz: xxx }. I see where you're coming from.

However, I don't think this means we should restrict 3rd party plugins to using this approach. If someone has a 3rd party plugin, I think it's up to them if they want to add an attribute to the config object or create a separate function to set the attribute. Plus, adding an attribute to the config object is already possible, rather the issue is the Typescript definitions and being able to augment the interface.

So, given that, I think this PR is beneficial to people working on customizing Day.js 👍

Sure I agree with you on the second part. but for the first example, It's on the author's choice of which way. I just recommend being more flexible on typing for 3rd parties while you can basically do anything with js prototypes. I already moved the config and solved my problem but overall I think it's beneficial.

@iamkun
Copy link
Owner

iamkun commented Jul 12, 2021

Agreed, cool

@iamkun iamkun merged commit 5a79cc6 into iamkun:dev Jul 12, 2021
iamkun pushed a commit that referenced this pull request Sep 10, 2021
## [1.10.7](v1.10.6...v1.10.7) (2021-09-10)

### Bug Fixes

* Add  Spanish (Mexico) [es-mx] locale ([#1614](#1614)) ([3393f2a](3393f2a))
* Add Arabic (Iraq) [ar-iq] locale ([#1627](#1627)) ([b5a1391](b5a1391))
* add format object type to type file ([#1572](#1572)) ([5a79cc6](5a79cc6))
* duration plugin when parsing duration from ISO string, set missing components to 0 instead of NaN ([#1611](#1611)) ([252585b](252585b))
* narrow type for `add` and `subtract` ([#1576](#1576)) ([1686962](1686962))
* update customParseFormat plugin strict x X parsing ([#1571](#1571)) ([08adda5](08adda5))
* update Lithuanian [lt] locale spelling for single month ([#1609](#1609)) ([255dc54](255dc54))
* Update Norwegian Bokmål [nb] local yearStart 4 ([#1608](#1608)) ([7a8467c](7a8467c))
* update plugin advancedFormat `isValid` validation ([#1566](#1566)) ([755fc8b](755fc8b))
* update Sinhalese [si] locale month name ([#1475](#1475)) ([63de2a8](63de2a8))
* update utcOffset plugin type file ([#1604](#1604)) ([f68e4b1](f68e4b1))
@iamkun
Copy link
Owner

iamkun commented Sep 10, 2021

🎉 This PR is included in version 1.10.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.10.7](iamkun/dayjs@v1.10.6...v1.10.7) (2021-09-10)

### Bug Fixes

* Add  Spanish (Mexico) [es-mx] locale ([#1614](iamkun/dayjs#1614)) ([3393f2a](iamkun/dayjs@3393f2a))
* Add Arabic (Iraq) [ar-iq] locale ([#1627](iamkun/dayjs#1627)) ([b5a1391](iamkun/dayjs@b5a1391))
* add format object type to type file ([#1572](iamkun/dayjs#1572)) ([5a79cc6](iamkun/dayjs@5a79cc6))
* duration plugin when parsing duration from ISO string, set missing components to 0 instead of NaN ([#1611](iamkun/dayjs#1611)) ([252585b](iamkun/dayjs@252585b))
* narrow type for `add` and `subtract` ([#1576](iamkun/dayjs#1576)) ([1686962](iamkun/dayjs@1686962))
* update customParseFormat plugin strict x X parsing ([#1571](iamkun/dayjs#1571)) ([08adda5](iamkun/dayjs@08adda5))
* update Lithuanian [lt] locale spelling for single month ([#1609](iamkun/dayjs#1609)) ([255dc54](iamkun/dayjs@255dc54))
* Update Norwegian Bokmål [nb] local yearStart 4 ([#1608](iamkun/dayjs#1608)) ([7a8467c](iamkun/dayjs@7a8467c))
* update plugin advancedFormat `isValid` validation ([#1566](iamkun/dayjs#1566)) ([755fc8b](iamkun/dayjs@755fc8b))
* update Sinhalese [si] locale month name ([#1475](iamkun/dayjs#1475)) ([63de2a8](iamkun/dayjs@63de2a8))
* update utcOffset plugin type file ([#1604](iamkun/dayjs#1604)) ([f68e4b1](iamkun/dayjs@f68e4b1))
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.10.7](iamkun/dayjs@v1.10.6...v1.10.7) (2021-09-10)

### Bug Fixes

* Add  Spanish (Mexico) [es-mx] locale ([#1614](iamkun/dayjs#1614)) ([3393f2a](iamkun/dayjs@3393f2a))
* Add Arabic (Iraq) [ar-iq] locale ([#1627](iamkun/dayjs#1627)) ([b5a1391](iamkun/dayjs@b5a1391))
* add format object type to type file ([#1572](iamkun/dayjs#1572)) ([5a79cc6](iamkun/dayjs@5a79cc6))
* duration plugin when parsing duration from ISO string, set missing components to 0 instead of NaN ([#1611](iamkun/dayjs#1611)) ([252585b](iamkun/dayjs@252585b))
* narrow type for `add` and `subtract` ([#1576](iamkun/dayjs#1576)) ([1686962](iamkun/dayjs@1686962))
* update customParseFormat plugin strict x X parsing ([#1571](iamkun/dayjs#1571)) ([08adda5](iamkun/dayjs@08adda5))
* update Lithuanian [lt] locale spelling for single month ([#1609](iamkun/dayjs#1609)) ([255dc54](iamkun/dayjs@255dc54))
* Update Norwegian Bokmål [nb] local yearStart 4 ([#1608](iamkun/dayjs#1608)) ([7a8467c](iamkun/dayjs@7a8467c))
* update plugin advancedFormat `isValid` validation ([#1566](iamkun/dayjs#1566)) ([755fc8b](iamkun/dayjs@755fc8b))
* update Sinhalese [si] locale month name ([#1475](iamkun/dayjs#1475)) ([63de2a8](iamkun/dayjs@63de2a8))
* update utcOffset plugin type file ([#1604](iamkun/dayjs#1604)) ([f68e4b1](iamkun/dayjs@f68e4b1))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants