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

can't find variable: document #35

Closed
nsendek opened this issue Jan 28, 2019 · 16 comments
Closed

can't find variable: document #35

nsendek opened this issue Jan 28, 2019 · 16 comments

Comments

@nsendek
Copy link

nsendek commented Jan 28, 2019

I'm trying to us the h3-js library in a react-native app. I npm installed the library, and then when I just import h3 into one of my files, it gives me an error that say's can't find variable document. I'm assuming it is coming from the libh3.js file in "./dist/out" where it has the function....

var libh3 = (function() {
var _scriptDir = typeof document !== 'undefined' && dtocument.currentScript ? document.currentScript.src : undefined;
return (
........

I don't know where the problem is coming from but it's not in my code because I'm just trying to import h3-js

@nrabinowitz
Copy link
Collaborator

I haven't tried h3-js in React Native, and I'm not very familiar with the environment. The Emscripten-generated code checks for the environment using fairly simple duck-typing, and it assumes that the environment is Node, a browser, or a Web Worker. My guess here is that the React Native environment passes the "browser" test (which I think looks for the existence of window) but doesn't have document or other browser features.

I'm not sure what to recommend here, since this seems likely to be an issue with Emscripten itself. The unsatisfactory answer is that we don't currently support React Native :/.

@Benassou
Copy link

Benassou commented Jul 19, 2019

Hey

I have found a workaround. If I comment out the following code in node_modules/h3-js/dist/browser/h3-js.js

if (document.currentScript) {
  scriptDirectory = document.currentScript.src;
}

Module["setWindowTitle"] = function (title) {
  document.title = title;
};

everything works as expected. This does feel quite hacky so I was wondering if there was a better way to remove the document dependency? It doesn't seem to have any real functional use in the code.

@nrabinowitz
Copy link
Collaborator

AFAIK, this is not used - it's part of the Emscripten boilerplate. I can see whether there's a clean way to remove this in the build, but in the meantime, it might be less hacky to polyfill document with {} at the global level (not sure how to do this in the React Native - you'd need to attach it to whatever the equivalent of the global object is in that context). That at least avoids modifying the imported lib, which could make it hard to upgrade, etc.

@Benassou
Copy link

OK, thanks. Right now I'm still trying out this library but I'll be sure to test your solution and report back.

@dakkafex
Copy link

any update on this?

@Benassou
Copy link

I haven't gone into production yet so I'm still using the hack. The polyfill solution does sound promising though.

@tylergaw
Copy link

tylergaw commented Oct 25, 2019

I hit this today too and have a workaround that seems fine. In RN you can use global to set variables that all scripts have access to. You can set a document key on that. Here's what I'm doing in my app:

In App.js

global.document = {};

export default App = () => (...)

then in AnyFile.js import and use h3 as normal

import { h3ToGeo } from "h3-js";
const hexID = geoToH3(longitude, latitude, 7);

@nrabinowitz
Copy link
Collaborator

There's a fork here that fixes the distro package for React Native.

I'm closing this for now, as RN support isn't in scope for h3-js at the moment.

@len-art
Copy link

len-art commented Apr 2, 2020

I have the same problem on a web app. I'm trying to use h3-js in a web worker.

Uncaught ReferenceError: document is not defined

Should I open a new issue?

@tylergaw
Copy link

tylergaw commented Apr 2, 2020

@len-art I don't think web workers have a global document object. You could probably do the same work-around that I posted above where you create that object before you use h3. Something like:

let document = {};

// some code using h3
// the rest of your web worker code

I haven't done that, but that's all h3 needs. It doesn't need any members of document, only expects it to be there.

@len-art
Copy link

len-art commented Apr 3, 2020

Workers don't have a global document, yes. I tried doing what you suggested but have some problems because webpack and worker-loader probably work a bit differently than a vanilla inclustion of worker code.
But that's something I have to solve myself, it's not an H3 problem, so thanks for the help.

@benjamintd
Copy link

@len-art have you figured out how to solve this issue in a webpack-compiled webworker? I am facing the same problem and was not able to solve it easily bu setting document to {}.

@len-art
Copy link

len-art commented Oct 21, 2020

@benjamintd sorry for responding so late: No, I haven't. I went another route and implemented something different which, in retrospect, made more sense than using H3. Regardless, I would really love it if the authors would fix this problem. There are a ton of use cases for H3 that benefit from being calculated in a separate thread.

@benjamintd
Copy link

Thanks @len-art for following up! I ended up going the ugly way and using patch-package, which allows patching your node_modules on build, with the following patch:

Because we use h3-js in a webworker, we need to delete this condition to avoid a crash
due to the emscripten compilation, which bugs when the `window` exists but not `document`.

diff --git a/node_modules/h3-js/dist/browser/h3-js.js b/node_modules/h3-js/dist/browser/h3-js.js
index fa31223..f722cb4 100644
--- a/node_modules/h3-js/dist/browser/h3-js.js
+++ b/node_modules/h3-js/dist/browser/h3-js.js
@@ -24,10 +24,6 @@ var libh3 = function (libh3) {
   var readAsync;

   {
-    if (document.currentScript) {
-      scriptDirectory = document.currentScript.src;
-    }
-
     if (scriptDirectory.indexOf("blob:") !== 0) {
       scriptDirectory = scriptDirectory.substr(0, scriptDirectory.lastIndexOf("/") + 1);
     } else {

I would gladly have sent a PR to h3-js directly, but it seems that it would involve changing the docker image that runs emscripten build. I did not have time to dig further.

@markusand
Copy link

@benjamintd sorry for responding so late: No, I haven't. I went another route and implemented something different which, in retrospect, made more sense than using H3. Regardless, I would really love it if the authors would fix this problem. There are a ton of use cases for H3 that benefit from being calculated in a separate thread.

There's definitely plenty of use cases where using h3-js in web workers makes total sense. Something apparently simple like the following is blocking the execution of main thread, making the map unresponsible during 3/4 seconds (neither being able to show a spinner).

import { cellToBoundary, polygonToCells } from 'h3-js';
import { polygon as toPolygon } from '@turf/turf';
import type { Slope } from './types';

onmessage = (event: MessageEvent<{ slopes: Slope[], resolution: number }>) => {
  const { slopes, resolution } = event.data;

  const cells = slopes.flatMap(slope => {
    const { coordinates, type } = slope.feature.geometry;
    const polygons = type === 'MultiPolygon' ? coordinates : [coordinates];
    const h3Ids = polygons.flatMap(polygon => polygonToCells(polygon, resolution, true));
    return h3Ids.map(id => toPolygon([cellToBoundary(id, true)], { id }));
  });

  postMessage(cells);
};

I've tried to just fake a global document object as suggested, without success. Just for context, I'm using Vite.
Has it been any movement in this after the update to v4?

@nrabinowitz
Copy link
Collaborator

Has it been any movement in this after the update to v4?

Sorry, this is still on my TODO list. It's likely solved by a more recent version of Emscripten, but I believe that's still blocked on performance regressions, making this a potentially difficult update.

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

No branches or pull requests

8 participants