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

Only add to window if window exists #42

Merged
merged 1 commit into from
Nov 23, 2020

Conversation

SamSamskies
Copy link
Contributor

@SamSamskies SamSamskies commented Sep 6, 2020

This prevents window is not defined error when using SSR frameworks such as Next.js.

Sample Next.js app that demonstrates error when trying to import suggestions module: https://github.com/SamSamskies/suggestions-demo

Screenshot of error:
Screen Shot 2020-09-08 at 2 36 04 PM

This prevents `window is not defined` error when using SSR frameworks such as Next.js.
@utb2017
Copy link

utb2017 commented Nov 20, 2020

how do i "npm install suggestions" with this pull request? next js deployment installs the version without this change and errors out my build

@SamSamskies
Copy link
Contributor Author

how do i "npm install suggestions" with this pull request? next js deployment installs the version without this change and errors out my build

That's actually exactly why I made this PR. I think you can import this library using Next.js dynamic imports to get around the issue for now. https://nextjs.org/docs/advanced-features/dynamic-import#with-no-ssr.

You could also fork this repo and merge this PR in your fork and use your forked version. Last release for this library was 13 months ago, so I'm not sure if it's being maintained anymore.

@utb2017
Copy link

utb2017 commented Nov 20, 2020

I could not get around the issue with the dynamic import (works on dev, but vercel rebuilds the node_modules folder based on package.json). The vercel builder still throws an error on deployment. I was able to deploy my website through firebase hosting though. Here is a link to a simple deployment method if you are a firebase user. NextJS and Firebase Hosting

Use firebase deploy --only hosting at the end of this video.

@SamSamskies
Copy link
Contributor Author

I could not get around the issue with the dynamic import (works on dev, but vercel rebuilds the node_modules folder based on package.json)

What do you mean by this? You shouldn't be editing the files inside of the node_modules folder. The only time I'd do this is when trying to debug issues with dependencies.

Specifying { ssr: false } as an option to your dynamic import should get around the window is not defined issue because then the import will only execute in the browser.

@tristen tristen merged commit 5649df6 into tristen:gh-pages Nov 23, 2020
@tristen
Copy link
Owner

tristen commented Nov 23, 2020

Thanks for the PR!

@tristen
Copy link
Owner

tristen commented Nov 23, 2020

Published v1.7.1

SamSamskies added a commit to SamSamskies/mapbox-gl-geocoder that referenced this pull request Nov 24, 2020
This fixes the `window is not defined` issue when using `mapbox-gl-geocoder` with an SSR app. PR in the `suggestions` library that fixes the issue tristen/suggestions#42.
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 this pull request may close these issues.

3 participants