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

fix: fix isDayjs check logic #2383

Merged
merged 4 commits into from
Jul 27, 2023
Merged

fix: fix isDayjs check logic #2383

merged 4 commits into from
Jul 27, 2023

Conversation

iamkun
Copy link
Owner

@iamkun iamkun commented Jul 21, 2023

ref: https://github.com/moment/moment/blob/develop/src/lib/moment/constructor.js#L78

isDayjs should return true in different versions of dayjs.

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #2383 (0f2089d) into dev (061aa7e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               dev     #2383   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          183       183           
  Lines         2198      2200    +2     
  Branches       593       594    +1     
=========================================
+ Hits          2198      2200    +2     
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)

@iamkun iamkun merged commit 5f3f878 into dev Jul 27, 2023
@avand
Copy link

avand commented Aug 15, 2023

@iamkun I'm experiencing what I think is a bug that this ticket may resolve. When I use dayjs.isDayjs() together with a plugin like dayjs.extend(utc) and organize my code in multiple files, I get false when I expect `true. Let me demonstrate:

// utcDay.js
import dayjs from 'dayjs';
import utc from 'dayjs/plugin/utc';

function utcDay() {
  dayjs.extend(utc);
  return dayjs.utc();
}
// index.js
import utcDay from 'utcDay';

dayjs.isDayjs(utcDay()) // => false

Would the changes you made here fix this issue? And if so, when will they be released?

Update: I just pointed my package.json to GitHub (e.g., "dayjs": "https://github.com/iamkun/dayjs") and this issue went away. So it indeed seems to have resolved my issue.

github-actions bot pushed a commit that referenced this pull request Sep 19, 2023
## [1.11.10](v1.11.9...v1.11.10) (2023-09-19)

### Bug Fixes

* Add Korean Day of Month with ordinal ([#2395](#2395)) ([dd55ee2](dd55ee2))
* change back fa locale to the Gregorian calendar equivalent ([#2411](#2411)) ([95e9458](95e9458))
* duration plugin - MILLISECONDS_A_MONTH const calculation ([#2362](#2362)) ([f0a0b54](f0a0b54))
* duration plugin getter get result  0 instead of undefined ([#2369](#2369)) ([061aa7e](061aa7e))
* fix isDayjs check logic ([#2383](#2383)) ([5f3f878](5f3f878))
* fix timezone plugin to get correct locale setting ([#2420](#2420)) ([4f45012](4f45012))
* **locale:** add meridiem in `ar` locale ([#2418](#2418)) ([361be5c](361be5c))
* round durations to millisecond precision for ISO string ([#2367](#2367)) ([890a17a](890a17a))
* sub-second precisions need to be rounded at the seconds field to avoid adding floats ([#2377](#2377)) ([a9d7d03](a9d7d03))
* update $x logic to avoid plugin error ([#2429](#2429)) ([2254635](2254635))
* Update Slovenian locale for relative time ([#2396](#2396)) ([5470a15](5470a15))
* update uzbek language translation ([#2327](#2327)) ([0a91056](0a91056))
@github-actions
Copy link

🎉 This PR is included in version 1.11.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

BePo65 pushed a commit to BePo65/dayjs that referenced this pull request Sep 20, 2023
## [1.11.10](iamkun/dayjs@v1.11.9...v1.11.10) (2023-09-19)

### Bug Fixes

* Add Korean Day of Month with ordinal ([iamkun#2395](iamkun#2395)) ([dd55ee2](iamkun@dd55ee2))
* change back fa locale to the Gregorian calendar equivalent ([iamkun#2411](iamkun#2411)) ([95e9458](iamkun@95e9458))
* duration plugin - MILLISECONDS_A_MONTH const calculation ([iamkun#2362](iamkun#2362)) ([f0a0b54](iamkun@f0a0b54))
* duration plugin getter get result  0 instead of undefined ([iamkun#2369](iamkun#2369)) ([061aa7e](iamkun@061aa7e))
* fix isDayjs check logic ([iamkun#2383](iamkun#2383)) ([5f3f878](iamkun@5f3f878))
* fix timezone plugin to get correct locale setting ([iamkun#2420](iamkun#2420)) ([4f45012](iamkun@4f45012))
* **locale:** add meridiem in `ar` locale ([iamkun#2418](iamkun#2418)) ([361be5c](iamkun@361be5c))
* round durations to millisecond precision for ISO string ([iamkun#2367](iamkun#2367)) ([890a17a](iamkun@890a17a))
* sub-second precisions need to be rounded at the seconds field to avoid adding floats ([iamkun#2377](iamkun#2377)) ([a9d7d03](iamkun@a9d7d03))
* update $x logic to avoid plugin error ([iamkun#2429](iamkun#2429)) ([2254635](iamkun@2254635))
* Update Slovenian locale for relative time ([iamkun#2396](iamkun#2396)) ([5470a15](iamkun@5470a15))
* update uzbek language translation ([iamkun#2327](iamkun#2327)) ([0a91056](iamkun@0a91056))
BePo65 pushed a commit to BePo65/dayjs that referenced this pull request Sep 23, 2023
## [1.11.10](iamkun/dayjs@v1.11.9...v1.11.10) (2023-09-19)

### Bug Fixes

* Add Korean Day of Month with ordinal ([iamkun#2395](iamkun#2395)) ([dd55ee2](iamkun@dd55ee2))
* change back fa locale to the Gregorian calendar equivalent ([iamkun#2411](iamkun#2411)) ([95e9458](iamkun@95e9458))
* duration plugin - MILLISECONDS_A_MONTH const calculation ([iamkun#2362](iamkun#2362)) ([f0a0b54](iamkun@f0a0b54))
* duration plugin getter get result  0 instead of undefined ([iamkun#2369](iamkun#2369)) ([061aa7e](iamkun@061aa7e))
* fix isDayjs check logic ([iamkun#2383](iamkun#2383)) ([5f3f878](iamkun@5f3f878))
* fix timezone plugin to get correct locale setting ([iamkun#2420](iamkun#2420)) ([4f45012](iamkun@4f45012))
* **locale:** add meridiem in `ar` locale ([iamkun#2418](iamkun#2418)) ([361be5c](iamkun@361be5c))
* round durations to millisecond precision for ISO string ([iamkun#2367](iamkun#2367)) ([890a17a](iamkun@890a17a))
* sub-second precisions need to be rounded at the seconds field to avoid adding floats ([iamkun#2377](iamkun#2377)) ([a9d7d03](iamkun@a9d7d03))
* update $x logic to avoid plugin error ([iamkun#2429](iamkun#2429)) ([2254635](iamkun@2254635))
* Update Slovenian locale for relative time ([iamkun#2396](iamkun#2396)) ([5470a15](iamkun@5470a15))
* update uzbek language translation ([iamkun#2327](iamkun#2327)) ([0a91056](iamkun@0a91056))
ohsory1324 pushed a commit to ohsory1324/dayjs that referenced this pull request Dec 20, 2023
const IS_DAYJS = '$isDayjsObject'

// eslint-disable-next-line no-use-before-define
const isDayjs = d => d instanceof Dayjs || !!(d && d[IS_DAYJS])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a problem related to this change. We have a Dayjs object that we store in location.state where it get's serialized. Upon retrieving said object we use isDayjs to check whether the object is actually of type Dayjs or if we have to parse it, something along the lines of

const maybeBrokenDayjs(obj: any) => {
  if (isDayjs(obj) {
    return obj;
  } else if (obj?.$d) {
    return dayjs(obj.$d)
  }
}

This now obviously breaks as isDayjs will suddenly yield true. While this is of course easily fixable it is a breaking change and thus quite unexpected in a Patch-Release :(

@iamkun iamkun deleted the fix/is-dayjs branch April 28, 2024 14:22
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