-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat:add plugin objectSupport #887
Conversation
add/set/subtract support Object parameter and constructor support Object parameter
Codecov Report
@@ Coverage Diff @@
## dev #887 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 165 +1
Lines 1341 1385 +44
Branches 281 295 +14
=========================================
+ Hits 1341 1385 +44
Continue to review full report at Codecov.
|
src/plugin/objectSupport/index.js
Outdated
oldParse.bind(this)(cfg) | ||
} | ||
|
||
proto.setObject = function (argument) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems no need to put it on the proto
, just a simple function will be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right.
Help us keeping 100% test coverage https://github.com/iamkun/dayjs/blob/dev/CONTRIBUTING.md |
|
types/plugin/objectSupport.d.ts
Outdated
declare module 'dayjs' { | ||
interface Dayjs { | ||
set(argument: object): Dayjs | ||
add(argument?: object): Dayjs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a ?
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I was careless.processed.
sorry, I was careless
src/plugin/objectSupport/index.js
Outdated
*/ | ||
const parseObjectArgument = (d, utc) => parseArrayArgument([ | ||
0, | ||
d.y || d.year || d.years || 1970, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check our this.$utils().p()
prettyUnit to better handle this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Their role is different, d
is input by the user
d => { years: 2010, months: 1, days: 14, hours: 15, minutes: 25, seconds: 50, milliseconds: 125 }
or
d => { y: 2010, M: 1, d: 14, h: 15, m: 25, s: 50, ms: 125 }
I don't have to judge units, just need to get the value of some units, just like the REGEX_PARSE in constant.js.
This code is similar to the part in index.js where parseDate handles strings.
const d = date.match(C.REGEX_PARSE)
In the index.js 60 lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/iamkun/dayjs/blob/dev/src/plugin/duration/index.js#L42
Something like this to process all kinds of user input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that right? But it doesn't support date-> dayjs({date:1})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 you can get prettyUnit
from this.$utils().p
instead of important it directlly.
2 you may have to extend a new prettyUnit
function to treat day and date key both mean day-of-the-month.
like https://github.com/iamkun/dayjs/blob/dev/src/plugin/duration/index.js#L26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that all right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, seniors, let me learn a lot.
This is my first PR. Thanks♪(・ω・)ノ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are most welcome, cheers.
src/plugin/objectSupport/index.js
Outdated
$d.millisecond || 0 | ||
] | ||
if (utc) return new Date(Date.UTC(...arr)) | ||
return new Date(...arr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please avoid using this ...arr
advanced es6 syntax, this will hugely increase the bundle size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems much smaller now in packed
src/plugin/objectSupport/index.js
Outdated
|
||
const oldParse = proto.parse | ||
proto.parse = function (cfg) { | ||
// console.log(cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no test code, pls.
src/plugin/objectSupport/index.js
Outdated
return chain | ||
} | ||
|
||
const subtractObject = function (argument) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these three functions could be combined into one function with different arguments
return oldAdd.bind(this)(number, string) | ||
} | ||
const oldSubtract = proto.subtract | ||
proto.subtract = function (number, string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
substract
could be implemented as negative add
, subtract { day: 1 } => add { day: 1 * -1 }, so that we could only care add
and set
logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the three methods have been merged into one.
src/plugin/objectSupport/index.js
Outdated
} | ||
|
||
const oldSet = function (int, string) { | ||
return this.clone().$set(string, int) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a copy-paste instead of proto.set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I've changed the set
to have the same parameter location as add
, otherwise you have to judge in callObject
, which is inefficient and increases the package size.
or set and add can't share a callObject
.
also, my attempt to use proto.set
in oldSet
will cause a dead loop because proto.set
is redirected.
Using 'proto.$set' has the problem of the 'this' scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, why useing return proto.set
will cause a dead loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const oldSet = function (int, string) {
return proto.set(string, int)
}
...
proto.set = function (string, int) {
...
}
his code causes oldSet
to enter the newly defined proto.set
each time, So it creates a dead loop.
In the new code, I direct used a callback to solve this
pointing problem.
## [1.8.27](v1.8.26...v1.8.27) (2020-05-14) ### Bug Fixes * Add Kinyarwanda (rw) locale ([#903](#903)) ([f355235](f355235)) * Add plugin objectSupport ([#887](#887)) ([52dfb13](52dfb13)) * Add Turkmen (tk) locale ([#893](#893)) ([a9ca8dc](a9ca8dc)) * Fix CustomParseFormat plugin set locale error ([#896](#896)) ([8035c8a](8035c8a)) * Fix locale month function bug ([#908](#908)) ([bf347c3](bf347c3)) * Update CustomParseFormat plugin to support Array formats ([#906](#906)) ([97856c6](97856c6))
🎉 This PR is included in version 1.8.27 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [1.8.27](iamkun/dayjs@v1.8.26...v1.8.27) (2020-05-14) ### Bug Fixes * Add Kinyarwanda (rw) locale ([#903](iamkun/dayjs#903)) ([f355235](iamkun/dayjs@f355235)) * Add plugin objectSupport ([#887](iamkun/dayjs#887)) ([52dfb13](iamkun/dayjs@52dfb13)) * Add Turkmen (tk) locale ([#893](iamkun/dayjs#893)) ([a9ca8dc](iamkun/dayjs@a9ca8dc)) * Fix CustomParseFormat plugin set locale error ([#896](iamkun/dayjs#896)) ([8035c8a](iamkun/dayjs@8035c8a)) * Fix locale month function bug ([#908](iamkun/dayjs#908)) ([bf347c3](iamkun/dayjs@bf347c3)) * Update CustomParseFormat plugin to support Array formats ([#906](iamkun/dayjs#906)) ([97856c6](iamkun/dayjs@97856c6))
## [1.8.27](iamkun/dayjs@v1.8.26...v1.8.27) (2020-05-14) ### Bug Fixes * Add Kinyarwanda (rw) locale ([#903](iamkun/dayjs#903)) ([f355235](iamkun/dayjs@f355235)) * Add plugin objectSupport ([#887](iamkun/dayjs#887)) ([52dfb13](iamkun/dayjs@52dfb13)) * Add Turkmen (tk) locale ([#893](iamkun/dayjs#893)) ([a9ca8dc](iamkun/dayjs@a9ca8dc)) * Fix CustomParseFormat plugin set locale error ([#896](iamkun/dayjs#896)) ([8035c8a](iamkun/dayjs@8035c8a)) * Fix locale month function bug ([#908](iamkun/dayjs#908)) ([bf347c3](iamkun/dayjs@bf347c3)) * Update CustomParseFormat plugin to support Array formats ([#906](iamkun/dayjs#906)) ([97856c6](iamkun/dayjs@97856c6))
## [1.8.27](iamkun/dayjs@v1.8.26...v1.8.27) (2020-05-14) ### Bug Fixes * Add Kinyarwanda (rw) locale ([#903](iamkun/dayjs#903)) ([f355235](iamkun/dayjs@f355235)) * Add plugin objectSupport ([#887](iamkun/dayjs#887)) ([52dfb13](iamkun/dayjs@52dfb13)) * Add Turkmen (tk) locale ([#893](iamkun/dayjs#893)) ([a9ca8dc](iamkun/dayjs@a9ca8dc)) * Fix CustomParseFormat plugin set locale error ([#896](iamkun/dayjs#896)) ([8035c8a](iamkun/dayjs@8035c8a)) * Fix locale month function bug ([#908](iamkun/dayjs#908)) ([bf347c3](iamkun/dayjs@bf347c3)) * Update CustomParseFormat plugin to support Array formats ([#906](iamkun/dayjs#906)) ([97856c6](iamkun/dayjs@97856c6))
add/set/subtract support Object parameter
and constructor support Object parameter
The related unit tests have been added.