contentful / ui-extensions-sdk

A JavaScript library to develop custom apps for Contentful
https://www.contentful.com/developers/docs/extensibility/app-framework/
MIT License
125 stars 31 forks source link

`window` used directly in library breaks SSR environments #1046

Closed r00dY closed 2 years ago

r00dY commented 2 years ago

Hey,

https://github.com/contentful/ui-extensions-sdk/blob/5dfb775ac0e708136ac7fcff839b639552668e06/lib/index.ts#L13

By using window "on root level" developers that build extension with some SSR framework (nextjs, nuxt, Gatsby, etc) have troubles. It's because on server-part window is not available and there are errors because of that.

Solution: there should be simple condition that takes into account that window might be undefined. It's gonna make lives of a lot of devs easier :)

This issue is example of this: https://github.com/contentful/ui-extensions-sdk/issues/344 - the solution works, but it makes extension load slower and hence user experience is worse and developers must build complex workarounds with lazy loading contentful SDK into their apps which is completely unnecessary.

Jwhiles commented 2 years ago

@r00dY can you explain what you would expect to happen in the case where there is no window is accessible? Fundamentally the purpose of this library is to allow the transfer of data from the Contentful User Interface into an iframe and back again. And as a result it's not clear to me what the use case is for calling the init method in an SSR context.

andipaetzold commented 2 years ago

Closing because of inactivity. Feel free to reopen if you are still running into issues.

jeffyamada commented 2 years ago

@Jwhiles it could just do nothing, or bail - this only ever happens on server for SSR (not applicable for a Contentful Extension of course, but in my case and many others I suspect, the extension lives on a server used for the frontend as well.

I'm not super familiar with how the sdk works - but I can't run it at all on this Next.js project because simply importing it causes it to fail completely

r00dY commented 2 years ago

@Jwhiles sorry for silence.

It's very simple. I have ecommerce in next.js and want to quickly create custom Contentful extension. The easiest way for me to do this is to create new page in my next app (/__contentful-field-extension) - this page will be Contentful extension. Setting up totally separate environment only for the purpose of small Contentful extension is annoying.

Frameworks like next.js are "SSR-first" and you can't prevent next from doing SSR. And it doesn't apply only to next, it applies to most modern next-like frameworks (Gatsby, nuxt).

The fix is super simple, you must assume that window can be undefined and let the app build. If window is undefined you can assume you're on the server and you can do "nothing". It would help a lot.

petergray-maker commented 2 years ago

@andipaetzold @Jwhiles I too am hitting the same issue and I feel that it should be fixed on contentfuls side and not force the break of SSR. Otherwise you're forcing the issue over to developers to apply the same work around again and again, when it could be simply fixed where the problem actually arises.

As @r00dY points out, it would really help out a lot! Many thanks

andipaetzold commented 2 years ago

Hey folks,

this issue has been resolved with our latest release (v4.8.1). When importing the App SDK, window should not be used anymore and youshouldn't see any error in Node. Please reopen this ticket if you are running into the same problem.

r00dY commented 2 years ago

@andipaetzold appreciate this, thank you! 💪

andipaetzold commented 2 years ago

@r00dY did this change work for you? I am waiting for some positive feedback before shipping this to everyone as it changes some internal flows.