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

query-engine-node-api: delete getConfig() #3608

Merged
merged 1 commit into from
Jan 19, 2023
Merged

Conversation

tomhoule
Copy link
Contributor

@tomhoule tomhoule commented Jan 18, 2023

The Prisma CLI now uses getConfig from the @prisma/prisma-fmt-wasm package. This method is not used anymore.

We also observe a small size improvement:

Before:

-r-xr-xr-x 2 root root 14311096 Jan 1 1970 libquery_engine.node

After:

-r-xr-xr-x 2 root root 14282424 Jan 1 1970 libquery_engine.node

amounting to a ~30kb smaller node-api build of the query engine.

closes https://github.com/prisma/client-planning/issues/225

The Prisma CLI now uses getConfig from the @prisma/prisma-fmt-wasm
package. This method is not used anymore.

We also observe a small size improvement:

Before:

-r-xr-xr-x 2 root root 14311096 Jan  1  1970 libquery_engine.node

After:

-r-xr-xr-x 2 root root 14282424 Jan  1  1970 libquery_engine.node

amounting to a ~30kb smaller node-api build of the query engine.
@tomhoule tomhoule added this to the 4.10.0 milestone Jan 18, 2023
@tomhoule tomhoule merged commit 7e6991a into main Jan 19, 2023
@tomhoule tomhoule deleted the qe-node-api-no-get-config branch January 19, 2023 12:40
@jkomyno
Copy link
Contributor

jkomyno commented Jan 20, 2023

As discovered while working on prisma/prisma#17443, the Client runtime still uses getConfig() from both libquery (library) and query-engine (binary). Moving the Client runtime to @prisma/prisma-fmt-wasm doesn't work out of the box and would cost at least ~3MB to the Client. We should revert this change as agreed with @tomhoule and @millsp.

tomhoule added a commit that referenced this pull request Jan 20, 2023
We deleted getConfig from the node api builds of the query-engine for a
30kb gain with the assumption that it wasn't used at all anymore in the
CLI. It is not the case.

This reverts commit 7e6991a.
tomhoule added a commit that referenced this pull request Jan 20, 2023
We deleted getConfig from the node api builds of the query-engine for a
30kb gain with the assumption that it wasn't used at all anymore in the
CLI. It is not the case.

This reverts commit 7e6991a.
@janpio
Copy link
Contributor

janpio commented Jan 20, 2023

Reverted in #3619

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