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

[Discussion] User external h3 #2818

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
2 participants
@Pessimistress
Copy link
Contributor

Pessimistress commented Mar 21, 2019

This change avoids installing and bundling h3-js if the layers are not used, and also allows users to install an alternative implementation of h3 (e.g. an earlier version from the internal module)

Change List

  • Add required h3 props to H3HexagonLayer and H3ClusterLayer.

Pessimistress added some commits Mar 20, 2019

@Pessimistress Pessimistress requested review from nrabinowitz and ibgreen Mar 21, 2019

@ibgreen
Copy link
Contributor

ibgreen left a comment

Fine, solves the problem, but...

This dependency injection does create potential implications for other layers:

  • Is this going to be a generic pattern in similar situations?
  • Are we going to do the same for S2Layer?
  • Are we going to manage external deps like this more generally etc?

Practical issue

  • These layers can no longer just be injected into the deck.gl JSON module. Special plumbing needs to be done to make them work.

I wonder if it is time for h3-js to become a monorepo and publish an @h3/core module that only contains the minimal necessary functions. Or maybe the bulk of the 150KB is in the core projection code? @nrabinowitz @isaacbrodsky

@Pessimistress

This comment has been minimized.

Copy link
Contributor Author

Pessimistress commented Mar 21, 2019

Or maybe the bulk of the 150KB is in the core projection code?

libh3.js (the EMSCRIPTEN transpiled code) is 147K and minified.

@Pessimistress

This comment has been minimized.

Copy link
Contributor Author

Pessimistress commented Mar 21, 2019

Regarding the S2Layer:

  • As far as I can tell, there is no versioning in the S2 system, while h3-js is only v3.
  • s2-geometry as a dependency is only 13.5KB minified and 3.7KB gzipped.
  • s2-geometry can be imported with a <script> tag, which creates a global S2 object. We exclude this module from the standalone bundle and look for the global object instead. There is no similar way to use H3.

@Pessimistress Pessimistress deleted the external-h3 branch Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.