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: navigator is not defined. #1202

Merged
merged 2 commits into from May 11, 2024
Merged

Conversation

thePeeyush
Copy link
Contributor

@thePeeyush thePeeyush commented Feb 16, 2024

Fix Issue #1165: Navigator is not defined.

Cause: Today many people use NextJS for building webapps. But when they importing Peer from peerjs they get this error because when webpack compile it in server side, navigator is not present there.

Issue: link

Changes Made:

In peerjs/dist/bundel.mjs : I made change in this file as given below literally remove the error from compiling and building in simple import without using useeffect or dynamic import. Only 'use client' is defined at top.

if (typeof navigator !== 'undefined') {
            this.isIOS = ["iPad", "iPhone", "iPod"].includes(navigator.platform)
        }
        else this.isIOS = false;

So this means, checking the navigator in support.ts is able to solve this problem.

Now : readonly isIOS = ["iPad", "iPhone", "iPod"].includes(navigator.platform);

Change to : readonly isIOS = typeof navigator !== "undefined" ? (["iPad", "iPhone", "iPod"].includes(navigator.platform)) : false;;

The navigator.platform only works when navigator is available else isIOS assigned false.

Reviewer Requests:

@jonasgloning : Please review the changes and provide feedback.

@irgalamarr
Copy link
Member

Hi @thePeeyush!

Thank you for the PR! I went over all previous issues and discussions about the problem. Upon reviewing the codebase, I noticed several other window and navigator references, which could potentially impact SSR in a similar manner.

Did I get it right from the discussion that using a local build of PeerJS with these changes was all you needed for a smooth SSR build for your projects? After, in runtime, it works correct?

@irgalamarr irgalamarr self-requested a review February 25, 2024 00:42
@thePeeyush
Copy link
Contributor Author

thePeeyush commented Feb 25, 2024

Hello @irgalamarr mam ,

Thanks for the discussion, I built this lib by modifying these changes and made a new npm package called airsharejs for testing. And it works completely fine without encountering any errors in vercel build and local build.

With PeerJS:

Screenshot 2024-02-25 213139

With airsharejs ( modified peerjs):

Screenshot 2024-02-25 213244

And it worked correctly in runtime. I made a file-sharing app in NextJS with airsharejs pkg and it runs smoothly in all browsers. You can checkout
Link: https://air0.vercel.app/

So it helps all NextJS devs to make apps with peerjs, I can check for build on other SSR frameworks if you want?

@irgalamarr irgalamarr added the investigating currently being looked into label Feb 28, 2024
@jonasgloning jonasgloning merged commit 4b7a74d into peers:master May 11, 2024
3 of 8 checks passed
github-actions bot pushed a commit that referenced this pull request May 11, 2024
## [1.5.3](v1.5.2...v1.5.3) (2024-05-11)

### Bug Fixes

* navigator is not defined. ([#1202](#1202)) ([4b7a74d](4b7a74d)), closes [#1165](#1165)
* remove need for `unsafe-eval` ([3fb31b3](3fb31b3))
Copy link

🎉 This PR is included in version 1.5.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigating currently being looked into released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants