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

Tom/deparse PR #63

Draft
wants to merge 6 commits into
base: 16-latest
Choose a base branch
from
Draft

Tom/deparse PR #63

wants to merge 6 commits into from

Conversation

pyramation
Copy link
Collaborator

#32 is the original PR, working here to clean up protobuf issues

@pyramation
Copy link
Collaborator Author

pyramation commented May 3, 2024

NOTE: @tomquist I'm thinking how to address 4.8M that proto.js adds, since this is literally bundling C objects already 30M, maybe it's not much add, but I'm going to consider tree-shaking for the deparser

Screen Shot 2024-05-02 at 6 28 01 PM

@pyramation
Copy link
Collaborator Author

another thought, once we get the supabase builds in CI/CD, I think we can get rid of those .a and .lib files

https://github.com/launchql/libpg-query-node/actions/workflows/build-supabase-artifacts.yml

@benasher44
Copy link
Contributor

I was also thinking instead of supabase builds, could we just publish the binaries to npm? esbuild for example does this and publishes the builds for different os/cpu variants as sub-packages, and then you can tweak your local yarn/npm settings to that only the ones relevant to the architectures your app supports are pulled in.

@benasher44
Copy link
Contributor

Looks like they just add them all as optional dependencies: https://github.com/evanw/esbuild/blob/main/npm/esbuild/package.json

@benasher44
Copy link
Contributor

benasher44 commented May 4, 2024

Then these package.json attributes tell package managers whether they're relevant on the current arch: https://github.com/evanw/esbuild/blob/main/npm/%40esbuild/linux-arm/package.json#L14

And then install.js is generated here, which is run by a postinstall script in the main package: https://github.com/evanw/esbuild/blob/main/npm/esbuild/package.json#L10. That script makes various attempts to resolve the path to the binary here (either by the already-installed optional deps or fetching from/via npm)

Anyway the point is that it seems like there's a general path here to publishing as separate packages to npm followed by an install script that resolves the binary path (no need to rely on supabase)

@gregnr gregnr mentioned this pull request May 28, 2024
@pyramation
Copy link
Collaborator Author

@benasher44 it's a good idea! I would just need an example of how to fetch from npm cdn without using npm install, is that possible?

@benasher44
Copy link
Contributor

Yep can you see here: https://github.com/evanw/esbuild/blob/886c5456d4edc6094391e9e6fcccb19661621868/lib/npm/node-install.ts#L218

If configured correctly, npm/yarn should automatically get the relevant optional dependencies (based on declared CPU arch, etc.), but if npm/yarn hasn't done that, this method is available to fetch from npm

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