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

project(plugin-devtools): Fix fatal crash when using SSR #53

Closed
wants to merge 1 commit into from

Conversation

Owain94
Copy link

@Owain94 Owain94 commented Dec 20, 2023

Currently the dev tools plugin will try to register on nay platform causing the following error when using SSR (or prerendering)

ERROR ReferenceError: window is not defined

@zuriscript
Copy link
Owner

Hi @Owain94
Thank you very much for your contribution and your efforts to push for ssr/ssg compatibility 😊
Your fix is a step in the right direction, but there's a catch: it ties the useDevtools factory function to an injection context. This might not cover all use cases and could potentially lead to errors in different scenarios.

I think that a more robust approach involves extending our feature detection function beyond simply checking for the browser extension hook in the window variable. By using a try-catch block, we could handle the absence of global variables on the server. To optimize performance, we could even use a memoization technique to prevent throwing an error for every store.

Additionally, we should apply the same strategy to other plugins that rely on browser features, such as the persistence plugin (local storage/session storage).

What are your thoughts on this? Would you be interested in collaborating on implementing these enhancements?

@zuriscript zuriscript closed this Jan 4, 2024
@zuriscript
Copy link
Owner

Hi @Owain94
I have implemented feature detection for SSR myself in the meantime in #64 since this will be a great asset for the next release. Nevertheless, I am still interested to hear what you think about my concerns for #54. Maybe there could be a case for a SSR state transfer plugin.

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.

2 participants