oddsdk / ts-odd

An SDK for building apps with decentralized identity and storage.
https://odd.dev/
Apache License 2.0
178 stars 24 forks source link

ReferenceError: document is not defined - when in worker loading from cdn #411

Closed gotjoshua closed 2 years ago

gotjoshua commented 2 years ago

Summary

https://github.com/fission-codes/webnative/pull/404/files#r950738813

Problem

Following instructions for WebWorker

// const webnativeCDNURL = 'https://unpkg.com/webnative@0.32.0/dist/index.umd.min.js'
// tried both @latest and "@next"
const webnativeCDNURL = 'https://unpkg.com/webnative@0.34.0-alpha1//dist/index.umd.min.js'
const importRes = self.importScripts(webnativeCDNURL)

Error: ReferenceError: document is not defined

Impact

I can't use wn in web worker

Solution

different packaging / publishing strategy?

Screen Shot 2022-08-20 at 10 00 22 PM

icidasset commented 2 years ago

It's because of https://github.com/fission-codes/webnative/blob/8f50d7894630b4afa5a1d1af801c3b37a1826a9f/src/ipfs/config.ts#L49 We need to use importScripts instead when using a web worker.

Temporary workaround should be something like this:

import { IPFSPackage } from "webnative/ipfs/types.js"

importScripts(webnative.ipfs.DEFAULT_CDN_URL) // inserts into global scope
const ipfsPkg = self.IpfsCore as IPFSPackage

webnative.ipfs.set(
  await webnative.ipfs.nodeWithPkg(ipfsPkg)
)
gotjoshua commented 2 years ago

Thanks @icidasset !

something like this

Quite impressively functional untested code - it works!


suggested edit on the import line:


import type { IPFSPackage } from "webnative/ipfs/index"
gotjoshua commented 2 years ago

update on this workaround.

If i reload the page (and thus reload the worker) I get this:

VM9 index.min.js:76 Uncaught (in promise) Error: Key 'self' already exists
    at _3.importPeer (VM9 index.min.js:76:431831)
    at async fk.loadKeychain (index.min.js:77:84079)
    at async ede (index.min.js:77:285184)
    at async ZOe (index.min.js:77:283241)
    at async Om.start (index.min.js:77:283100)
    at async Object.epe (index.min.js:77:336598)
    at async RO (index.ts:121:26)
    at async initializeWnfsFromWorkerMsg (fa78af80-60ff-4017-9e2d-b14638b0a1df:20231:11)
    at async handleWebnativePermissionsFromUI (fa78af80-60ff-4017-9e2d-b14638b0a1df:20246:14)
    at async self.onmessage (fa78af80-60ff-4017-9e2d-b14638b0a1df:21049:13)

Screen Shot 2022-08-22 at 6 33 39 PM

is there a way to test first if this webnative.ipfs.set(... is needed?

icidasset commented 2 years ago

@gotjoshua Good to know! We're still working on the next release.

gotjoshua commented 2 years ago

this may have been do to a double initiation race condition (so not on reload, but on second call during the same webnative.ipfs.set from the same worker... i guess this workaround won't be needed after your release, but i'm still curious about how to test if the ipfs node is already wired up and working.

icidasset commented 2 years ago

but i'm still curious about how to test if the ipfs node is already wired up and working.

You should only need to set it once. There's also await webnative.ipfs.get() to get the active IPFS instance, but the downside of that is that it'll create a new IPFS node with the default setup if there hasn't been created one yet.

gotjoshua commented 2 years ago

downside of that

yeah, read that and avoided it.

could be ergonomic to add webnative.ipfs.get(skipCreation=false) flag... so that it can serve as a test that could return undefined if not yet setup.

( at one point maybe i'll set up webnative repo for contributing, so this suggestion could be accompanied with a PR, but not there yet... staying with the @demanding_enduser role a bit longer )

gotjoshua commented 2 years ago

Is this work around needed anymore?

icidasset commented 2 years ago

@gotjoshua Should be fixed in stable 0.34.0 release.

gotjoshua commented 2 years ago

Temporary workaround should be something

I can confirm this workaround is no longer needed as of 0.34.0