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

Incompatible with vite #14

Closed
fc opened this issue Oct 3, 2022 · 6 comments · Fixed by #19
Closed

Incompatible with vite #14

fc opened this issue Oct 3, 2022 · 6 comments · Fixed by #19

Comments

@fc
Copy link

fc commented Oct 3, 2022

When trying to include @macrostrat/cesium-martini in a bare bones Vite project the following error occurs:

browser-external:path:9 Module "path" has been externalized for browser compatibility. Cannot access "path.join" in client code.
get @ browser-external:path:9
18:12:23.384 index.cjs:14 Uncaught ReferenceError: __dirname is not defined
    at node_modules/cesium/index.cjs (index.cjs:14:3)
    at __require2 (chunk-7FP5O474.js?v=6ac06d1a:10:50)
    at node_modules/@macrostrat/cesium-martini/dist/index.js (index.cjs:16:1)
    at __require2 (chunk-7FP5O474.js?v=6ac06d1a:10:50)
    at dep:@macrostrat_cesium-martini:1:16

You can see this in a very basic repro here (check your browser's dev console):
https://stackblitz.com/edit/vitejs-vite-pqnxx6?file=package.json,src%2FApp.jsx&terminal=dev

By hacking the cesium module to import the cesium module directly to bypass the error (import cesium using cesium/Build/Cesium/Cesium instead of cesium), then I see this error:

Uncaught ReferenceError: regeneratorRuntime is not defined
    at worker-farm.ts:52:3
    at worker-farm.ts:52:3
    at node_modules/@macrostrat/cesium-martini/dist/index.js (worker-farm.ts:52:3)
    at __require2 (chunk-7FP5O474.js?v=4361fb93:10:50)
    at dep:@macrostrat_cesium-martini:1:16

It seems like I can workaround that with this - yarn add -D regenerator-runtime then:

import 'regenerator-runtime/runtime';
// import cesium-martini module here

note: using the @vitejs/plugin-legacy didn't appear to work

I think this can be resolved by adding @babel/plugin-transform-runtime to your project's babelrc

@davenquinn
Copy link
Owner

Thanks, this is a helpful report.

I think the first part might be a Cesium issue? There are a few gotchas with bundling Cesium certainly.

I've struggled with the regeneratorRuntime issues for a while in a few modules, and I think that I've managed to fix them in a few cases by switching bundling from Rollup to Parcel.js. Not sure if that would work in this case.

@fc
Copy link
Author

fc commented Oct 3, 2022

For the Cesium problem, at a glance it seems like a Cesium issue.

This is a somewhat similar problem someone encountered for that problem:
CesiumGS/cesium#9212 (comment)

Their solution was to essentially resolve the cesium import to point to cesium/Build/Cesium/Cesium instead which bypasses the calls to path/dirname.

I just setup a standalone vite project that imports cesium to see what happens and it does not encounter the same error.
I don't understand enough of the internals of how vite works but maybe since cesium is a dependency of cesium-martini it is loaded differently in vite? Aha... check this out:
https://github.com/CesiumGS/cesium/blob/main/package.json#L32-L33

  "main": "index.cjs",
  "module": "./Source/Cesium.js",

Could it be that because cesium-martini is compiled as CommonJS, it loads the CJS version of Cesium. However, since I'm running vite it will try to load everything as a module. Maybe there needs to be an ESM version of cesium-martini to resolve this particular issue?

For the regeneratorRuntime issue, I just tried a few things to get it working but having some difficulty in just doing the test. I tried adding it to the babelrc but that didn't seem to resolve it and resulted in a new error 🤷 .

I think if this is added to the top of cesium-martini/src/index.ts, it might resolve it but only a hunch for now:

import "regenerator-runtime/runtime";

@fc
Copy link
Author

fc commented Nov 2, 2022

In addition to the fix in the PR, I also added this resolution in my package.json which may or may not apply to others situations

"resolutions": {
    "cesium": "^1.93.0",
}

Ref: yarn resolutions

@fc
Copy link
Author

fc commented Nov 4, 2022

Just saw on your Twitter you have been busy with another project - congrats!
Do you know when you may come back to this? Definitely can fork to meet my more immediate needs but less ideal

@fc
Copy link
Author

fc commented Nov 9, 2022

For anyone who might need it, a fork is available here.
https://github.com/fc/cesium-martini/tree/fc/esm

@fc
Copy link
Author

fc commented Nov 15, 2022

just an fyi that in that branch I also needed to set the package manager in the package json. I'm not sure why this wasn't needed before but may be something that you may run into.
fc@1a17841

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

Successfully merging a pull request may close this issue.

2 participants