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

CometD does not start when bundled with rollup and rollup-plugin-commonjs #863

Open
bidorffOL opened this issue May 17, 2019 · 6 comments

Comments

Projects
None yet
3 participants
@bidorffOL
Copy link

commented May 17, 2019

Ever since #858, I'm getting a Cannot read property 'cometdRuntime' of undefined when attempting to initialize CometD. I believe that it may be because I'm bundling CometD into my app using rollup and rollup-plugin-commonjs.

Shouldn't this patch be using var runtime = root['cometdRuntime']; instead of var runtime = this['cometdRuntime']; or check if this exists ?

@sbordet

This comment has been minimized.

Copy link
Member

commented May 17, 2019

@bidorffOL this is being passed as an argument in the immediate invocation of the function.
For formal parameter that receives that argument is called root.
What environment are you using, where this is not defined?
In NodeJS, root and this will point to different objects (one to the module context, the other to the global context).

@bidorffOL

This comment has been minimized.

Copy link
Author

commented May 17, 2019

I'm running the app in the browser. However, I'm using rollup to bundle my app as an 'IIFE' , this may be the reason why this is undefined.

@sbordet

This comment has been minimized.

Copy link
Member

commented May 17, 2019

@bidorffOL CometD also uses IIFE, and this is defined.
Can you give me a small project that reproduces the error you have?

@bidorffOL

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

I'm sorry for replying so late, here is a small example where you can reproduce the behavior (the error is shown in the devtools when browser-sync starts up).
(BTW, the error can't be reproduced with CometD <4.0.4)

dirk-olmes added a commit to dirk-olmes/cometd-issue-863 that referenced this issue Jun 21, 2019

@dirk-olmes

This comment has been minimized.

Copy link

commented Jun 21, 2019

I'm stumbling over the same issue, albeit I'm using rollupjs directly not via gulp. I have also put together a small project to demo my use case.

@sbordet

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

I'm out next week, but I will take a look and fix this issue (or rollback to previous code).
Thanks for the repro projects!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.