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

chore: Upgrade to loaders.gl@4.0 #2394

Merged
merged 1 commit into from
Oct 24, 2023
Merged

chore: Upgrade to loaders.gl@4.0 #2394

merged 1 commit into from
Oct 24, 2023

Conversation

ibgreen
Copy link
Collaborator

@ibgreen ibgreen commented Oct 23, 2023

loaders.gl 4.0 brings in foundations for ongoing binary arrow table work

@igorDykhta
Copy link
Collaborator

igorDykhta commented Oct 24, 2023

It looks like certain geojson tests are failing when parseSync is called:

parseSync("POLYGON ((-74.387589 40.632238, -74.387589 40.632238))")

**ReferenceError: Response is not defined**
    at validHTTPResponse (/Desktop/unfolded/kepler.gl/node_modules/@loaders.gl/core/dist/index.cjs:625:23)
    at selectLoaderSync (/Desktop/unfolded/kepler.gl/node_modules/@loaders.gl/core/dist/index.cjs:579:8)
    at parseSync (/Desktop/unfolded/kepler.gl/node_modules/@loaders.gl/core/dist/index.cjs:1052:18)

Was the Response object provided by polyfills before? Kepler.gl is still using node 14. And the loaders.gl upgrade guide mentions that Details: The expectation is that loaders.gl v4.0+ will be used with Node.js version 18 and higher

@ibgreen
Copy link
Collaborator Author

ibgreen commented Oct 24, 2023

It looks like certain geojson tests are failing when parseSync is called:

parseSync("POLYGON ((-74.387589 40.632238, -74.387589 40.632238))")

**ReferenceError: Response is not defined**
    at validHTTPResponse (/Desktop/unfolded/kepler.gl/node_modules/@loaders.gl/core/dist/index.cjs:625:23)
    at selectLoaderSync (/Desktop/unfolded/kepler.gl/node_modules/@loaders.gl/core/dist/index.cjs:579:8)
    at parseSync (/Desktop/unfolded/kepler.gl/node_modules/@loaders.gl/core/dist/index.cjs:1052:18)

Was the Response object provided by polyfills before? Kepler.gl is still using node 14. And the loaders.gl upgrade guide mentions that Details: The expectation is that loaders.gl v4.0+ will be used with Node.js version 18 and higher

Response should still be polyfilled fir node version < 18.
https://github.com/visgl/loaders.gl/blob/master/modules/polyfills/src/index.ts#L108-L114.

I guess possible reasons are:

  • the bundler might fail to pull in the node version of the polyfills index file.
  • the node version test fails for some reason
  • ...

How to debug further? Maybe edit the polyfills index file in node_modules and add some logging etc.

Also I am updating to beta.7 which does have some fixes, let's see.

@ibgreen
Copy link
Collaborator Author

ibgreen commented Oct 24, 2023

BTW, do we know why kepler is still on Node 14? Let's make a test and see where the problems are.

@igorDykhta
Copy link
Collaborator

Also I am updating to beta.7 which does have some fixes, let's see.

Looks like beta.7 fixed failing tests.

@ibgreen
Copy link
Collaborator Author

ibgreen commented Oct 24, 2023

Hmm let me rebase and squash and push with signed commit

Signed-off-by: Ib Green <ib@foursquare.com>
@ibgreen ibgreen merged commit b54c173 into master Oct 24, 2023
@delete-merged-branch delete-merged-branch bot deleted the ib/loaders4 branch October 24, 2023 16:35
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 this pull request may close these issues.

3 participants