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

Bundling @turf/convex with Webpack not working due to concaveman dependency #2191

Closed
braco opened this issue Aug 8, 2021 · 12 comments
Closed
Labels
bug bundling Anything related to issues with bundling @turf/convex

Comments

@braco
Copy link

braco commented Aug 8, 2021

@turf/concave is erroring with Queue is not a constructor due to a longstanding issue with one of the dependencies:

mapbox/concaveman#18

https://github.com/Turfjs/turf/blob/master/packages/turf-convex/package.json#L62

"@turf/convex": "^6.5.0",
  const points = featureCollection([
    point([10.195312, 43.755225]),
    point([10.404052, 43.8424511]),
    point([10.579833, 43.659924]),
    point([10.360107, 43.516688]),
    point([10.14038, 43.588348]),
    point([10.195312, 43.755225]),
  ]);
  convex(points);
@JamesLMilner
Copy link
Collaborator

JamesLMilner commented Aug 8, 2021

Hi @braco, yes unfortunately this is a known issue on the concaveman project which effects us downstream. I believe the underlying issue is actually a problem with Webpack itself (mapbox/concaveman#18 (comment)). I wrote a response in that thread that explains how to circumvent it short term if you're using Webpack.

@braco
Copy link
Author

braco commented Aug 9, 2021

Thanks @JamesLMilner. I think that's a yarn only fix , without npm-force-resolutions or overrides? I switched to hull.js, which has been a much simpler fix.

@mourner
Copy link
Contributor

mourner commented Aug 9, 2021

Released concaveman v1.2.1 with the fix, hope that helps.

@JamesLMilner
Copy link
Collaborator

@mourner thanks! Appreciate that it wasn't the solution you were looking to resolve it with. Hope things go well with the ES rewrite.

I will look at raising a PR to use 1.2.1 and closing this.

@stebogit
Copy link
Collaborator

stebogit commented Aug 9, 2021

Hey guys, just to make clear, would this be an issue on @turf/concave or @turf/convex? The title of the issue and the example say two different things.

Regardless, I don't see any issue with either modules, at least not with the doc's example reported by @braco:

https://turf-sandbox.netlify.app/?gist=85c24aba86193f3542ae1493d90e986d
Screen Shot 2021-08-09 at 9 28 15 AM

@JamesLMilner
Copy link
Collaborator

JamesLMilner commented Aug 9, 2021

I think the title/text should be convex. The concaveman library is not used in concave : https://github.com/Turfjs/turf/blob/84110709afda447a686ccdf55724af6ca755c1f8/packages/turf-boolean-concave/package.json

@stebogit
Copy link
Collaborator

stebogit commented Aug 9, 2021

Still, I don't see the reported error. @braco can you please post a code snippet reproducing the error?

@JamesLMilner
Copy link
Collaborator

JamesLMilner commented Aug 9, 2021

@stebogit you would only see it if you were bundling with Webpack I think, as it's a specific issue between concaveman and Webpack

@JamesLMilner JamesLMilner changed the title @turf/concave broken due to concaveman dependency Bundling @turf/convex with Webpack not working due to concaveman dependency Aug 9, 2021
@JamesLMilner JamesLMilner added the bundling Anything related to issues with bundling label Sep 7, 2021
@JamesLMilner
Copy link
Collaborator

JamesLMilner commented Dec 5, 2021

Okay update here following closing #2200 and #2200

Unfortunately we can't upgrade to concaveman 1.2.1 which fixes the issues with Queue/rbush because rbush at version 3.0.1 which is an ESM module which we can't use in turf at the moment because there is no transpiliation step. The potential solutions are:

  • Vendor concaveman (not a huge fan of this for obvious reasons)
  • Create a separate module as per @turf/jsts or @turf/geojson-rbush
  • Kindly request concaveman uses the ES5 rbush (big ask, potentially not compatible with the maintainers needs)
  • Kindly request concaveman publishes an ES5 version, similar to how rbush is exposed under rbush/rbush (big ask)

@ChrisHSandN
Copy link

For visibility, the concaveman dependency update will fix the content security policy issues reported in #261, #1903, and #2233

@mourner
Copy link
Contributor

mourner commented Dec 6, 2021

because rbush at version 3.0.1 which is an ESM module which we can't use in turf at the moment because there is no transpiliation step

Can you expand more on this? I thought Turf transpiled/bundled all dependencies, but I'm not sure how it all works in a TS ecosystem. Generally, I would love to move more of my libraries to ESM-only ES6 entry point without transpilation, leaving the ES5 transpilation on downstream users if they need legacy compatibility.

@smallsaucepan
Copy link
Member

@turf/convex now incorporates the fixed 1.2.1 version of concaveman.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bundling Anything related to issues with bundling @turf/convex
Projects
None yet
Development

No branches or pull requests

6 participants