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

Shim ICAL to prevent using the global object #3682

Merged
merged 1 commit into from
Nov 23, 2021
Merged

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Nov 23, 2021

Fix #3649

The library ical.js heavily depends on instanceof checks which will break if two separate versions of the library are used (e.g. bundled one and global one).

Ref https://webpack.js.org/guides/shimming/

How to test

  1. Create some events in the near future
  2. Observe that the calendar widget is working fine
  3. Install notes app
  4. Observe that the calendar widget doesn't show any events anymore
  5. Apply my patch
  6. Observe that the calendar widget is working fine again

@st3iny st3iny added 3. to review Waiting for reviews bug labels Nov 23, 2021
@st3iny st3iny added this to the v2.4.0 milestone Nov 23, 2021
@st3iny st3iny self-assigned this Nov 23, 2021
The library ical.js heavily depends on instanceof checks which will break if
two separate versions of the library are used (e.g. bundled one and global
one).

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny changed the title Shim ICAL to prevent using the global object. Shim ICAL to prevent using the global object Nov 23, 2021
@st3iny
Copy link
Member Author

st3iny commented Nov 23, 2021

/backport to stable2.3

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request A backport was requested for this pull request label Nov 23, 2021
@codecov
Copy link

codecov bot commented Nov 23, 2021

Codecov Report

Merging #3682 (1cc13f4) into master (bfa60fb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3682   +/-   ##
=========================================
  Coverage     27.70%   27.70%           
  Complexity      123      123           
=========================================
  Files           165      165           
  Lines          6018     6018           
  Branches        877      877           
=========================================
  Hits           1667     1667           
  Misses         4351     4351           
Flag Coverage Δ
javascript 22.34% <ø> (ø)
php 94.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 bfa60fb...1cc13f4. Read the comment docs.

@raimund-schluessler
Copy link
Member

Hm, my solution back then was to "fix" ICAL.js, see nextcloud/tasks@d7a24b0 and https://github.com/raimund-schluessler/ical.js/commit/5d069324d17dd4ce1e214b7c7aab9ddf7521e6b5.

Might be worth checking if your definitely cleaner solution also works for Tasks.

@ChristophWurst ChristophWurst modified the milestones: v2.4.0, v3.0.0 Nov 23, 2021
@ChristophWurst
Copy link
Member

/backport to stable2.4

@st3iny st3iny merged commit 9309fa5 into master Nov 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/noid/shim-ical branch November 23, 2021 13:57
@backportbot-nextcloud
Copy link

The backport to stable2.3 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable2.4 failed. Please do this backport manually.

@ChristophWurst
Copy link
Member

/backport to stable2.4

@backportbot-nextcloud backportbot-nextcloud bot removed the backport-request A backport was requested for this pull request label Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calendar widget is empty after update to 22.2.2
3 participants