fix(great-circle): fix antimeridian splitting by upgrading arc to v1.0.0

[thomas-hervey]

Summary

Fixes #3030

Details

Routes crossing the international dateline (e.g. Tokyo-LAX, Auckland-LAX, Shanghai-SFO) were returning a malformed MultiLineString with a gap at the +/-180 boundary. This was caused by a bug in the GDAL-ported linear heuristic in arc.js v0.1.x/v0.2.x.

Fix

arc v1.0.0 replaces the heuristic with analytical bisection, producing exact +/-180 boundary coordinates and a correctly closed MultiLineString.

Changes:

• Bump arc dependency from ^0.2.0 to ^1.0.0
• Remove offset from Arc() call. Antimeridian splitting is now automatic
• Mark options.offset as deprecated in JSDoc (kept for backward compat)
• Remove unused offset destructuring

Test

• Add three antimeridian test fixtures: Tokyo-LAX, Auckland-LAX, Shanghai-SFO
• Add tape tests asserting MultiLineString output and exact +/-180 boundary coords
• Fix fixture-dependent tests to use named getFixture() lookup instead of fixtures[0]
• Regenerate basic.geojson snapshot (arc@1.0.0 defaults to 100 points)

When testing locally, arc@0.2.0 fails, but passes at arc@1.0.0

=== arc@0.2.0 ===
FAIL  Tokyo -> LAX: expected MultiLineString, got LineString
FAIL  Auckland -> LAX: gap at split -- last=[174.7900,-36.8500] first=[-176.0046,-29.7388]
FAIL  Shanghai -> SFO: expected MultiLineString, got LineString

found 0 vulnerabilities
=== arc@1.0.0 ===
PASS  Tokyo -> LAX: split at antimeridian, lat=47.3643
PASS  Auckland -> LAX: split at antimeridian, lat=-33.0522
PASS  Shanghai -> SFO: split at antimeridian, lat=52.3090
[rerun: b3]

... and the visual results were verified in geojson.io (see screenshot below)

---

Please provide the following when creating a PR:

• Meaningful title, including the name of the package being modified.
• Summary of the changes.
• Heads up if this is a breaking change. NO Breaking change
• Any issues this resolves.
• Inclusion of your details in the contributors field of package.json - you've earned it! 👏
• Confirmation you've read the steps for preparing a pull request.

[thomas-hervey]

@smallsaucepan @rowanwins I see that you've contributed more recently and was wondering if I could get your review on this PR. In short, it's a bump of the arc package to resolve an antimeridian splitting issue #3030 .

Thanks!

[smallsaucepan]

Thanks for this. While checking to see if this might fix the sourcemap issue #2721 as well noticed that arc as of 1.0.0 is ESM only?

[thomas-hervey]

@smallsaucepan Yes, arc@1.0.0 is ESM-only. I believe that this is safe for turf because tsup/esbuild bundles arc into turf's output at build time (all node versions passed CI build above). Is there a reason we'd still want to support CommonJS?

Regarding #2721, arc@1.0.0 also adds src/ (i.e., Typescript source files) to its published files alongside sourcemaps in dist/ with .js.map files, which may help there too, but probably needs a more thorough investigation, so LMK if you think I should jump on that.

[smallsaucepan]

We do currently support CJS and dropping it would mean a breaking change. Going ESM only is being touted for v8, though that's a little way off.

There might be some nuances that make it ok. However we got caught out with this recently (see #3018) so will want to be very sure.

Would you mind init-ing a simple CJS client project and require your PR to get an initial checkpoint?

[thomas-hervey]

Right, that makes sense. I will discuss with the other arc.js contributors on a plan forward.

[jgravois]

hi y'all 👋 arc.js co-maintainer here.

i'm not opposed to exporting CJS from arc.js directly if there's no other path forward, but i'm curious to investigate whether there's a viable way for this library to support ESM only dependencies and CJS consumers simultaneously if that would be welcome.

[jgravois]

i poked at this a little and it looks like one option would be to include arc itself in the @turf/great-circle bundle (CJS only) using the noExternal tsup flag.

export default [
  defineConfig({
    ...baseOptions,
    outDir: "dist/bundled",
    // since 'arc' is a ESM only dep, bundle its transpiled code in the CJS build 
    noExternal: ["arc"],
    format: "cjs",
  }),
  // ...
];

given that arc doesn't make use of a global cache or any singletons, this seems like a reasonable trade-off to me for legacy users, but i'm curious to hear what others think.

[thomas-hervey]

@jgravois I think your idea is the best approach so I started working on a solution locally but ran into an issue with a custom per-package config, so it's getting a bit messy.

To elaborate, I implemented a per-package tsup.config.ts for turf-great-circle that adds noExternal: ['arc'] on the CJS build only. So, arc's transpiled code gets inlined into dist/cjs/index.cjs at build time, eliminating the require('arc') call that fails at runtime. The ESM build is unchanged.

However, turf's monorepolint rule enforces that every package's build script must be specifically "tsup --config ../../tsup.config.ts", so a per-package config is blocked by the pre-commit hook.

I think that the alternative is to add noExternal: ['arc'] directly to the CJS format in the root tsup.config.ts:

defineConfig({
  ...baseOptions,
  outDir: "dist/cjs",
  format: "cjs",
  noExternal: ["arc"], // arc is ESM-only; inline for CJS consumers
}),

Since only @turf/great-circle imports arc, this would be a no-op for all other packages. That said, it feels like a package-specific concern in a shared config. I suspect @smallsaucepan would not want this top-level config change for just one package.

[jgravois]

i think it makes sense to open this PR for review tweaking the shared tsup config file and let the maintainers share their opinion.

personally I wouldn't bother trying to override the lint rule until/unless we receive feedback indicating that the overall approach is something they're interested in pursuing and that they do indeed want to sequester the override in a unique single package config.

[thomas-hervey]

@smallsaucepan following commit 762db6c to add noExternal: ["arc"], below details the state of CJS support with an example.

Per @jgravois's suggestion, the root tsup.config.ts now includes noExternal: ["arc"] on the CJS build only. This tells esbuild to inline arc's transpiled code into dist/cjs/index.cjs at build time, so no require('arc') call is emitted at runtime. The ESM build is unchanged arc remains external there.

CJS client test:

// plain CJS
const { greatCircle } = require("@turf/great-circle");

const cases = [
  { name: "Tokyo → LAX",    start: [139.7798,  35.5494], end: [-118.4085, 33.9416] },
  { name: "Auckland → LAX", start: [174.79,   -36.85],   end: [-118.41,   33.94]   },
  { name: "Shanghai → SFO", start: [121.81,    31.14],   end: [-122.38,   37.62]   },
];

for (const { name, start, end } of cases) {
  const result = greatCircle(
    { type: "Feature", geometry: { type: "Point", coordinates: start }, properties: {} },
    { type: "Feature", geometry: { type: "Point", coordinates: end },   properties: {} },
    { npoints: 10 }
  );
  const geom = result.geometry;
  const lastOfFirst   = geom.coordinates[0].at(-1);
  const firstOfSecond = geom.coordinates[1][0];
  console.log(`${geom.type === "MultiLineString" ? "PASS" : "FAIL"}  ${name}: ${geom.type}, split at ±180° (lat=${lastOfFirst[1].toFixed(4)})`);
}

Output against this PR's built dist/cjs/index.cjs:

PASS  Tokyo → LAX: MultiLineString, split at ±180° (lat=47.3643)
PASS  Auckland → LAX: MultiLineString, split at ±180° (lat=-33.0522)
PASS  Shanghai → SFO: MultiLineString, split at ±180° (lat=52.3090)

The built CJS file no longer contains require('arc'). I confirmed with grep "require.*arc" dist/cjs/index.cjs returning no matches. Happy to hear any concerns about the shared config change.

[mfedderly]

That's a neat fix for the ESM-only code using tsup. I'm curious for feedback on aiming for an esm-only v8 build over at this discussion #3058 if you get a chance.

[mfedderly]

I'm pretty sold on moving forward with v8 which is ESM only. I've got a bunch of work for that staged, and nobody has gotten on that discussion ticket to lodge a complaint about that path forward. I have added some testing (#3065) and it appears that the newest Node 20s and everything that has followed will work with ESM only. Additionally there are TypeScript consumer tests as well to try to make sure we are broadly compatible.

Because this changes the output and maybe someone was depending on the broken version of this, what I'd normally say is just a fix is perhaps more safely rolled out as a breaking change. I'm going to add this to the v8 milestone and revisit things once I've got the infrastructure moved around.