troika seems to break SSR

protectwise / troika

A JavaScript framework for interactive 3D and 2D visualizations

MIT License

1.6k stars 117 forks source link

troika seems to break SSR #46

Closed drcmda closed 4 years ago

drcmda commented 4 years ago

https://github.com/react-spring/drei/issues/25

https://github.com/react-spring/drei/issues/23

it seems to rely on document, which does not exist in node.

i believe in order to make troika pseudo-isomorphic all it would have to do is:

const linkEl = typeof document !== 'undefined' && document.createElement('a')

this would allow ssr to function normally.

lojjic commented 4 years ago

Thanks for the report. Am I correct in assuming that an SSR environment would not actually attempt to render the text? The suggested fix above would prevent an error on initial script module load, but would fail later on when it actually tried to use that linkEl, so I want to make sure that's not a problem.

lojjic commented 4 years ago

1b005ec should do the trick, but I'd feel better with some verification before I publish a 0.26.0 release.

drcmda commented 4 years ago

Yes that's correct, it just wouldn't crash. The client on the other hand has the chance to present a view immediately and it's rehydrated later on.

I'll ask them to try it out, thanks for the fix ...

lojjic commented 4 years ago

I've gone ahead and published 0.26.0 with that fix, I'm fairly confident that should fix the issue after testing it by require()-ing it in the node repl. Reopen this please if it doesn't solve the issue for your users. Thanks!

drcmda commented 4 years ago

thats awesome! thanks a lot, i'll re-publish my stuff as well.

gtolarc commented 4 years ago

Probably related, I am using nextjs and the following error occurs on startup. on v0.26

Troika createWorkerModule: web workers not allowed in current environment; falling back to main thread execution. ReferenceError: Worker is not defined
    at supportsWorkers (/Users/mk/dev/workspace/dapps/gione/node_modules/troika-worker-utils/dist/troika-worker-utils.umd.js:417:20)
    at defineWorkerModule (/Users/mk/dev/workspace/dapps/gione/node_modules/troika-worker-utils/dist/troika-worker-utils.umd.js:472:10)
    at /Users/mk/dev/workspace/dapps/gione/node_modules/troika-worker-utils/dist/troika-worker-utils.umd.js:606:30
    at /Users/mk/dev/workspace/dapps/gione/node_modules/troika-worker-utils/dist/troika-worker-utils.umd.js:2:66
    at Object.<anonymous> (/Users/mk/dev/workspace/dapps/gione/node_modules/troika-worker-utils/dist/troika-worker-utils.umd.js:5:2)

lojjic commented 4 years ago

@gtolarc That should just be a console.warn not a hard error, is it causing problems?

lojjic commented 4 years ago

@gtolarc I can see how that warning message would be very annoying in server logs, so I've gone ahead and suppressed it in non-browser environments: 3dedb8f2b338e9345c107831863152b115ca50d2

I've published 0.26.1 with that change.

gtolarc commented 4 years ago

yep, I haven't felt any other problems. and in 0.26.1, the warning message disappeared. Thank you! @lojjic

albert-schilling commented 3 years ago

Sorry, to open this up again. I still see the console.warning in my jest tests. Anyway to suppress this?

albert-schilling commented 3 years ago

I opened a tiny MR that checks if the process env is 'test' before console logging a failure in creating the web worker in troika-worker-utils: https://github.com/protectwise/troika/pull/85