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

Ical.js under CommonJS (Node.js) leaks global ICAL variable #329

Closed
moll opened this issue Jul 10, 2017 · 6 comments
Closed

Ical.js under CommonJS (Node.js) leaks global ICAL variable #329

moll opened this issue Jul 10, 2017 · 6 comments

Comments

@moll
Copy link

moll commented Jul 10, 2017

Hey,

Noticed some of my tests failing because they spot a global variable leakage. Turns out https://github.com/mozilla-comm/ical.js/blob/f3b8c7dc24a66424feb1735fe7ff38191e46ab99/lib/ical/helpers.js#L11 sets a global ICAL that is entirely unnecessary when running under CommonJS modules like when ran under Node.js. That ICAL variable should be scoped somehow to not set undesired globals. ;)

Cheers

@mschroeder mschroeder self-assigned this Jul 10, 2017
@kewisch
Copy link
Owner

kewisch commented Jul 10, 2017

The way globals are set bothered me too in the past, but changing it was quite some effort and is likely not backwards compatible. It looks like Martin is giving it another stab though!

@pscs
Copy link

pscs commented Feb 19, 2020

Is there any update on this? I've had problems using ical.js in Vue.JS in production mode because of the module loader (uncaught reference error). I've worked around the problem by changing line 11 to "var ICAL = module.exports; ", but that's obviously not nice for several reasons.

@mediaessenz
Copy link

mediaessenz commented May 20, 2021

I have exacly the same problem in conjuction with fullcalender.io's icalendar plugin, which uses this lib as well.
It took me hours to find the reason and using the same workaround than @pscs now, which i find quite ugly as well.

To help other people who have the same problem, here is the exact error, i got in the js console:
"Uncaught ReferenceError: assignment to undeclared variable ICAL"

To fix it, i changed line 11 of node_modules/ical.js/build/ical.js from

ICAL = module.exports;

to

var ICAL = module.exports;

@raimund-schluessler
Copy link

We ran into this issue as well when trying to use ical.js twice on the same page. We use ical.js for a Tasks and Calendar widget in Nextcloud and when both are enabled, ical.js breaks and doesn't parse correctly anymore, see nextcloud/tasks#1641 (comment). Adding var ICAL = ... fixes the issue for us.

Is there any possibility to get a fix for this problem into ical.js? Otherwise, we will have to use a fork with a hot fix for it, until it is resolved here, I guess.

@rocifier
Copy link

rocifier commented Jul 7, 2021

I'm also not able to use ical from @fullcalendar because of something related to this, it's a runtime error for me:

ical.js:11 Uncaught ReferenceError: ICAL is not defined
    at Object.UeCU (ical.js:11)
    at __webpack_require__ (bootstrap:84)
    at Module.e1AK (main.js:1)
    at __webpack_require__ (bootstrap:84)
    at Module.vzto (timetables.module.ts:2)
    at __webpack_require__ (bootstrap:84)
    at Module.Cxms (person-details.module.ts:2)
    at __webpack_require__ (bootstrap:84)
    at Module.ZAI4 (app.module.ts:1)
    at __webpack_require__ (bootstrap:84)

@kewisch
Copy link
Owner

kewisch commented Feb 14, 2022

#470 should fix most of this, the rest via es6 modules which is work in progress

@kewisch kewisch closed this as completed Feb 14, 2022
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.

7 participants