search

wulkano / Kap

An open-source screen recorder built with web technology
https://getkap.co
MIT License
18.12k stars 827 forks source link

Disable Next.js SSR #515

Open sindresorhus opened 6 years ago

sindresorhus commented 6 years ago

We make an app, not a website. SSR has no benefits to us, but lots of downsides. And it makes code more complicated.

divyanshu013 commented 6 years ago

Out of curiosity, what's the advantage of using Next.js for kap? Thanks!

albinekb commented 6 years ago

Super fast hot code reloading, free upgrades to latest Babel & webpack features, no custom webpack config.

sindresorhus commented 6 years ago

It also enforces good conventions and increases productivity.

bringking commented 6 years ago

@sindresorhus from my read of the code kap isn't using SSR, it is using a static export and electron-next to load the exported files. Can you give some more insight into what specifically you think should be disabled?

sindresorhus commented 6 years ago

@bringking It is definitely rendering on the server. We have to litter the code with const remote = electron.remote || false; all over the place so it won't crash when rendered on the server. https://github.com/wulkano/kap/blob/ef31a376ef63e0efc27de439b44a791f203fd1ed/renderer/pages/_app.js#L7

Also see:

https://github.com/wulkano/kap/blob/ef31a376ef63e0efc27de439b44a791f203fd1ed/renderer/pages/_app.js#L13-L21

bringking commented 6 years ago

@sindresorhus thanks for the clarification, I need to dig in more to understand SSR in the electron context

skllcrn commented 6 years ago

You might want to have a look at https://github.com/zeit/next.js/#3-with-no-ssr @bringking and @sindresorhus

theshortcut commented 6 years ago

🤔 if relying on dynamic imports to disable SSR that'd require moving all logic out of the renderer/pages files and have each page basically just dynamically import a component encapsulating what used to be on the page.

Kap will also have to roll something else (an App component?) to replace the built in functionality of _app.js right? That'll be difficult to work around since even if that was done, dropping that App component on each page won't have the same behavior as next's _app.js being a shared instance across all pages.

IssueHuntBot commented 5 years ago

@issuehunt has funded $80.00 to this issue.