-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Support for server side rendering #440
Comments
- adds check for `window` global variable - adds check for `navigator` global variable - also fixes missing `gulp-jslint` dependency Resolves issue airbnb#440
- adds check for `window` global variable - adds check for `navigator` global variable - also fixes missing `gulp-jslint` dependency - adds built files - patch version bump because no api level changes made Resolves issue airbnb#440
Thanks! I'll merge it in the next deploy. |
Since this issue is still open I wondered why this statement: Thank you and keep the good work up 👍 |
I don't want to pollute the global scope with the "inBrowser" variable. |
I am not 100% familiar with the architecture choices of bodymovin, (since I just started looking into the code), but @bodymovin, if you are afraid to pollute the global scope with the That way we can do: if (bodymovin.inBrowser()) {
// you are in the browser
} else {
// you are outside the browser
} |
@bodymovin I tried a webpack-workaround but unfortunately the server always breaks before anything else because of the navigator variable. @radiovisual that seems like a terrific idea |
Hey guys & @bodymovin , I am dealing with the same sort of issue. Any updates on the above? |
I added the bodymovin.inBrowser method on version 4.9.0 |
@irenama, can you show us how the server breaks when you tried your webpack workaround? @bodymovin, can you show me how you get setup in order to reproduce the error in #542, if you can step-by-step the reproducible steps you take to get to the error, I can take a look to see if I can help find a workaround for this issue and #542. |
Has there been any resolution to this? When I reference the npm module 4.10 I get this error. Here is the error produced:
And the file where bodymovin is referenced.
You will notice that I have commented out an import to a modified bodymovin js file which adds FYI I am using the boilerplate reactGo/reactGo |
@radiovisual I noticed when I was working on a webpack fix, that the build really breaks before I can even create a point where I can "mock" the navigator variable i.e. for the server-side where it's not known and should be ignored.. My Error is the one also @brtw got.
I'm also using a wrapper module |
We are also running into the
|
Is there a workaround for this issue? I am using vuejs, more specifically nuxt. The workaround seems to be implemented for the react version. |
Rather than adding inBrowser to the global scope, you can just inline inBrowser expression. (typeof navigator !== "undefined") && (function(root, factory) {
.... It solved the problem for me. I can do a PR but I think the IE fix PR should be merged first. |
I solved the problem for SSR |
@dardub Would you still be able to create a PR? This solved the issue for me also. Looks like the IE PR has been merged now. |
@NicolaSansom Sure. I haven't looked at this in a while so I thought maybe it had been fixed. |
any update about merging PR #905 ??? |
Is the PR itself being considered or is a different solution being discussed? |
I believe this issue can be closed, as #905 was merged. I can confirm that SSR is working for me with cc @bodymovin |
Yes, closing for now. |
Fix SSR rendering refs airbnb#440
Currently lacks checks for browser environment, am adding a PR to resolve this issue by checking if window and navigator is available before running
factory()
The text was updated successfully, but these errors were encountered: