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

Unable to get working in a Remix application #19

Closed
danecando opened this issue Mar 19, 2023 · 7 comments
Closed

Unable to get working in a Remix application #19

danecando opened this issue Mar 19, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@danecando
Copy link

danecando commented Mar 19, 2023

Description

Unable to initiate a call and pass it to DailyProvider in a Remix application. Since the error starts in DailyProvider I'm gonna post the issue here for now. I've tried running the same code in Vite and NextJS without any issues. When I try to use it in Remix I get errors:

react-dom.development.js:22839 Uncaught TypeError: Cannot read properties of undefined (reading 'call')
    at i2.value (daily-iframe-esm.js:1:121213)
    at DailyProvider.tsx:135:22
    at useDailyEvent.ts:47:5
    at commitHookEffectListMount (react-dom.development.js:23150:26)
    at commitPassiveMountOnFiber (react-dom.development.js:24926:13)
    at commitPassiveMountEffects_complete (react-dom.development.js:24891:9)
    at commitPassiveMountEffects_begin (react-dom.development.js:24878:7)
    at commitPassiveMountEffects (react-dom.development.js:24866:3)
    at flushPassiveEffectsImpl (react-dom.development.js:27039:3)

Details

"@daily-co/daily-js": "^0.40.0",
"@daily-co/daily-react": "^0.7.2",

Looking through the stack trace it appears to have something to do with using hydrateRoot in React 18. Very far from obvious what the specific issue is beyond that to me at least.

Reproduction

Here's a reproduction: https://stackblitz.com/edit/remix-daily-co?file=app/routes/index.tsx
And here's the same code in a regular react template working: https://stackblitz.com/edit/react-ts-2rur5u?file=App.tsx

@danecando danecando added the bug Something isn't working label Mar 19, 2023
@Regaddi
Copy link
Contributor

Regaddi commented Mar 20, 2023

Hey @danecando,

unfortunately the stackblitz link with the reproduction isn't accessible for me: it says Not Found.

Could you share a valid link for a reproduction? Thanks in advance!

@danecando
Copy link
Author

@Regaddi apologies just updated the link. I've also noted the same errors when letting DailyProvider/DailyCall setup the callObject

@Regaddi
Copy link
Contributor

Regaddi commented Mar 20, 2023

Thanks @danecando!

I started digging a bit and it seems like Remix is somehow interfering with the EventEmitter:
I can see that the EventEmitter prototype that's being referenced in the thrown error is missing the .off() method, but has the .removeListener() method defined, which sounds like Remix is bundled with an old version of EventEmitter.
I'll discuss this internally with the team and get back to you!

image

@nkrmr
Copy link

nkrmr commented Mar 23, 2023

@Regaddi
Hello, I have exactly the same issue with my integration of daily-react into my remix app.
And I don't found any solutions.

@Regaddi
Copy link
Contributor

Regaddi commented Mar 24, 2023

@danecando @nkrmr
I think I've found the issue:
remix-dev requires "@esbuild-plugins/node-modules-polyfill": "^0.1.4", which itself requires "rollup-plugin-node-polyfills": "^0.2.1", but that version is pretty old (released on Jun 14, 2019 and hasn't been maintained eversince).

So what happens is that the events module required by daily-js gets resolved with this old version of rollup-plugin-node-polyfills.

You can see that the 0.2.1 version doesn't include the off method:
https://github.com/ionic-team/rollup-plugin-node-polyfills/blob/master/polyfills/events.js

The latest version of a more recent fork of the same project has the .off method defined:
https://github.com/FredKSchott/rollup-plugin-polyfill-node/blob/v0.11.0/polyfills/events.js#L358-L360

I opened a PR at remorses/esbuild-plugins#36 to update the dependency, but that's more of a long-term solution. Also @remix-run/dev would have to update to the version of @esbuild-plugins/node-modules-polyfill that includes this fix.

I'm not familiar enough with Remix to provide a short-term solution, but I would probably try to add some configuration, that resolves an import to events with the rollup-plugin-polyfill-node module.

@nkrmr
Copy link

nkrmr commented Mar 27, 2023

@Regaddi

Thank you for your answer, there is a PR on Remix to update this plugin:
remix-run/remix#5274

I hope that fixes the issue.

Best regard

@Regaddi
Copy link
Contributor

Regaddi commented May 2, 2023

@nkrmr @danecando
This issue seems to be fixed with Remix 1.16.0 🎉
I forked the StackBlitz from the original comment, adjusted the Remix versions to use 1.16.0 and it doesn't throw an error anymore 🙌

@Regaddi Regaddi closed this as completed May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants