h3-js icon indicating copy to clipboard operation
h3-js copied to clipboard

React-native won't compile because of the use of h3-js

Open johhnsmmith198 opened this issue 4 years ago • 7 comments

importing h3-js this way import * as h3 from "h3-js";

would trigger the error below:

[Sat May 08 2021 16:50:27.116]  ERROR    Error: Requiring module "node_modules/h3-js/dist/browser/h3-js.js", which threw an exception: ReferenceError: Can't find variable: document
[Sat May 08 2021 16:50:27.132]  ERROR    ReferenceError: Can't find variable: document

I am wondering if this could be easily fixed? I am reading that there is a node version of h3-js, how can I import it instead of the browser version?

I tried to only import the function I was using at the time but I am still getting the same error

import {geoToH3} from 'h3-js';

I think the error is pointing to this code here in h3-js

  var readAsync;

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

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

johhnsmmith198 avatar May 08 '21 07:05 johhnsmmith198

See https://github.com/uber/h3-js/issues/35 - this is a known issue. We're not currently targeting React Native; I'd like this to work, obviously, but the issue is deep in the emscripten internals and hard to remove. There's a fork here that tries to address this, but I haven't had time to see if it can be fixed in the main library, and I don't have a great way to test the fix.

nrabinowitz avatar May 09 '21 23:05 nrabinowitz

A very simple way to fix it is to change the generated code in dist from:

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

to

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

Note that ENVIRONMENT_IS_WEB should be defined. I think it is generated by emscripten => https://github.com/emscripten-core/emscripten/issues/6717

I created an issue on their github

johhnsmmith198 avatar May 15 '21 05:05 johhnsmmith198

It looks like the bug on the Emscripten side (https://github.com/emscripten-core/emscripten/issues/14198) was fixed as of November of last year. Would it be possible to cut a new h3-js release with the latest emscripten, which hopefully will make this library React Native compatible?

corbt avatar May 24 '22 09:05 corbt

It looks like the bug on the Emscripten side (emscripten-core/emscripten#14198) was fixed as of November of last year. Would it be possible to cut a new h3-js release with the latest emscripten, which hopefully will make this library React Native compatible?

I'm trying to use h3-js in a web-worker and I have the same problem. It would be great if a new h3-js release was published with the latest emscripten version

padawannn avatar May 24 '22 16:05 padawannn

I will note that the reason why emscripten was not updated for this project was due to performance regressions with asm.js compilation on newer releases of emscripten than the version that is in use (likely due to asm.js being a less-common usage of emscripten these days).

Upgrading emscripten is not without trade-offs.

dfellis avatar May 24 '22 17:05 dfellis

I will note that the reason why emscripten was not updated for this project was due to performance regressions with asm.js

Oh interesting, I wasn't aware! Are there benchmarks for how large the impact would be on this library if it adopted the latest emscripten? Making this compatible with h3 would be really nice, but of course not at the cost of a significant performance regression for other users!

corbt avatar May 24 '22 19:05 corbt

It was a few years ago that we noticed the regression. I searched but couldn't find the benchmark we did back then. As I recall some functions were unaffected but most functions were 10-20% slower and a few were more than 50% slower but since it has literally been years in the meantime, it's probably a good idea to try again and re-benchmark against the current version of emscripten.

dfellis avatar May 25 '22 02:05 dfellis

I would also be interested in a version of h3-js that works on the server. I tried updating to the latest version of Emscripten to at least get a benchmark but I couldn't get it to work.

  • MODULARIZE_INSTANCE=1 is deprecated so you have to use MODULARIZE=1 instead, which returns a function which returns a promise that resolves to an instance.
  • Also I think you need to explicitly export _free now.

aduarterengifo avatar Oct 06 '22 20:10 aduarterengifo

I would also be interested in a version of h3-js that works on the server.

To be clear, h3-js works fine on a server running Node. It only has issues running in React Native.

nrabinowitz avatar Oct 06 '22 23:10 nrabinowitz

Sorry I misspoke. It doesn't work with vercel's edge runtime / middleware. Same error as react native.

ReferenceError: document is not defined

aduarterengifo avatar Oct 07 '22 17:10 aduarterengifo

Thanks for the clarification, that's good to know.

nrabinowitz avatar Oct 07 '22 17:10 nrabinowitz

This issue will bite any JavaScript runtime that has no global document. For Deno I found a workaround a while ago that involved dynamic import:

const document = {};
const h3 = await import("https://esm.sh/[email protected]");
const h3Index = h3.latLngToCell(37.3615593, -122.0553238, 7);
console.warn(h3Index);
// 87283472bffffff

cnrdh avatar Oct 21 '22 16:10 cnrdh

With Deno's recent support for npm packages, you may now use a regular import like

import * as h3 from "npm:[email protected]";

Easy enough, but non-standard and won't help people on other runtimes (and not even Deno Deploy as the feature is experimental). It would be great if h3-js could simply get rid of the document-reference.

cnrdh avatar Oct 21 '22 16:10 cnrdh

I've encountered this issue when trying to use h3 inside a web-worker as well; edited, I didn't see @padawannn 's comment :)

kenjinp avatar Jan 09 '23 20:01 kenjinp

Related discussion, with benchmarks for the emscripten upgrade issue: https://github.com/uber/h3-js/issues/163#issuecomment-1380986297

nrabinowitz avatar Jan 12 '23 20:01 nrabinowitz

This issue should be resolved with the most recent 4.1.0 release. I have been able to use the 4.1.0 dist directly in WebWorkers now greatly speeding up computations :)

4nthonylin avatar Jan 25 '23 07:01 4nthonylin

Thanks @4nthonylin! Closing for now - there may be other blockers for React Native or unusual environments, but hopefully this is a reasonable fix for many of the affected situations.

nrabinowitz avatar Jan 25 '23 23:01 nrabinowitz