Skip to content

Consolidate apache arrow imports #33

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

Merged

Conversation

hanbyul-here
Copy link
Contributor

Importing apache-arrow from root level outputs a lighter (~100kb) bundle. The assets built with subpaths imports included some of the classes and utilities (Vector, Data.. - which will lead to a problem with isInstanceof) that are supposed to come from apache-arrow.

I still think the problem should not be subpath import itself but might be the circular dependencies the subpath modules have - Rollup is trying to resolve the circular dependency by partially importing the package and then embedding the rest of it to the built asset to get out of circular dependency? - I am honestly not sure what exactly is happening here, but importing from root level seems to solve the issue.

@kylebarron
Copy link
Member

kylebarron commented Jan 2, 2025

This looks great! Would you be able to fix the lint errors? I figure you can just delete the now-unnecessary @ts-expect-error lines

@hanbyul-here hanbyul-here force-pushed the hanb/import-from-module-index branch from 43938f4 to 888f73c Compare January 6, 2025 21:23
Copy link
Member

@kylebarron kylebarron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@kylebarron kylebarron merged commit f1d6c93 into geoarrow:kyle/wkb Jan 6, 2025
@hanbyul-here hanbyul-here deleted the hanb/import-from-module-index branch January 7, 2025 15:03
danielfdsilva pushed a commit to danielfdsilva/geoarrow-js that referenced this pull request Feb 6, 2025
* Import from apache arrow root level

* Fix lint

* Consolidate imports to use the main one
kylebarron pushed a commit that referenced this pull request Feb 6, 2025
* Import from apache arrow root level

* Fix lint

* Consolidate imports to use the main one

Co-authored-by: Hanbyul Jo <[email protected]>
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.

2 participants