@graphql-codegen/client-preset-swc-plugin panics with out of bounds memory access #9057
Comments
|
I think it should be fixed by better error handling when the GraphQL operation is wrong. That will be fixed in the above mentioned PR. Edit: We found the cause, it was a Rust update that broke the SWC plugin! #9060. |
|
@JesseVelden thanks for authoring such a useful plugin! I'm get the same error on 0.1.2 of the new package and latest of the "old" package. I'm using nextjs 12.3.x new old |
|
@awinograd Can you please share a reproduction repository? |
|
My newly scaffolded nextjs app in a turborepo actually just stops working completely? Running If I remove the plugin from my Details
ready - started server on 0.0.0.0:8080, url: http://localhost:8080 info - Thank you for testing |
|
Yea, for me I've created a new issue for that, as |
|
@n1ru4l please find the MRE here https://github.com/awinograd/graphql-codegen-client-preset-swc-oom It turns out the offending next.config options are If OR If |
|
Hey, thanks @aplr for reporting the issue, @awinograd for providing a MRE, and @StampixSMO for the extra context on a very different behavior of just completely stopping to work with no errors. So I've investigated the issue using all of your helpful contexts, and it turned out that the reason why @JesseVelden's The error message/behavior is indeed incredibly bad, warn - You have enabled experimental feature (swcPlugins) in next.config.js.
warn - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.We are gonna downgrade our this is the PR that has the fix: #9155 |
|
can you please try out the new and let us know if it works or not, and please reopen the issue again if necessary |
|
Hey, big thanks to all of you for looking into this, even though it's considered experimental. I've tried out The signal changes between I'll create a MRE repo when I find time. |
|
Can this be re-opened? Also tried with |
|
Sorry for the delay in trying out the new version @YassinEldeeb . Was out on vacation but just got a chance to check it out. I'm still seeing errors in the MRE repro after bumping the version. I've updated the MRE with the bumped version |
## What's the purpose of this pull request? This PR makes `@faststore/core` compatible with SWC by dropping the `@faststore/graphql-utils` package, meaning we don't depend on a babel-only plugin built by us. ## How it works? ### Why we did it [SWC](https://swc.rs/) is a compiler and bundler used by Next.js. It is the default bundler since Next.js 12 the default code minifier since Next.js 13. It promises considerable **performance** improvements when compared to [babel](https://babeljs.io/), its main competitor. Our use of a custom babel plugin - made available through `@faststore/graphql-utils` - made us unable to use SWC, since babel plugins are not compatible with SWC. This plugin helped us optimize GraphQL queries, improve security of queries and mutations and handle GraphQL operations definitions. When this plugin was first introduced, there wasn't a solution that could useful to us at the community, so we had to develop it ourselves. Now, The Guild's [client-preset](https://the-guild.dev/graphql/codegen/plugins/presets/preset-client) does exactly what we need. It provides a all-in-one tool for handling persisted queries (optimization & security) and a GraphQL helper (`gql`) for defining operations, among other things we already used as standalone solutions, like type generation for the schema. By using `client-preset` we're no longer bound to babel or SWC exclusively, as their solution doesn't rely on any of them. There are recommended plugins for both platforms, as it helps reduce bundle sizes. We also helps reduce the code we have to maintain, adopt community standards and get up do date in terms of bundling and compilation tools. ### Results After the store was migrated to use `client-preset`, I ran a few builds to see if there would be any difference in bundle size. The results were not great, as the SWC version increased the bundle size considerably while reducing the build time in a negligible way (~5s reduction, although ~20% reduction percentage wise). I wasn't able to make the SWC plugin work, and I think that's because we're on Next 12. The support for SWC plugins within Next.js is still experimental (even in Next.js 13) and these errors are expected. It will probably work when we migrate to it, tho. I also ran a comparison considering the client-preset with the community babel plugin, and the results were comparable to the ones we had before, time-wise as well. **Click on the images** to read the data. | Before (babel, custom plugin) | After (SWC, no plugins) | After (babel, community plugin) | | ------------- | ------------- | ------------- | | <img src="https://github.com/vtex/faststore/assets/8127610/3c2b5dc1-e4ce-41b1-86fa-6e21d933c67a" width="400"> |<img src="https://github.com/vtex/faststore/assets/8127610/38a72e9f-f19c-487d-8124-f668b4c011a6" width="400"> | <img src="https://github.com/vtex/faststore/assets/8127610/b5064fd9-6de2-4cd1-8880-e7b570dc2a05" width="400"> |  |  |  | ### Implementation details #### operationName We previously used the operation name to identify queries and mutations on the backend. `client-preset` creates hashes based on the operation name to do the same thing. I updated the code to comply with this change, but added the operation name to the query arguments so 1. It could be provided to `envelop` on the `execute` function 2. It could be used for debugging when inspecting network calls using the browser dev tools To do that, I used `client-preset` `onExecutableDocumentNode` hook and extracted it manually at the `codegen.ts` config file. #### @generated dir Previously, the schema and other GraphQL files were inside the `@generated/graphql` folder. I changed it to `@generated` directly so `@generated/graphql` imports wouldn't break. It would be a pain to change all files, and we don't currently have other generated files other than graphql files, so it made sense to change it. #### SWC minification I enabled SWC minification at `next.config.js` since it is already the default minification tool Next 13. It provided awesome results, decreasing the build time in 10s to 15s. ## How to test it? This is a breaking change, as we know use the `gql` helper function and how to use it. We already exported it from `@faststore/core/api`, but it was possible to import it from `@faststore/graphql-utils` as well. Importing it from `@faststore/core/api` is the only possible and recommended way from now on. At the code, users also have to change the `gql` call to an actual function call: ```ts // Before const query = gql`query here` // After const query = gql(`query here`) ``` This change only affects stores containing API Extensions. Users shouldn't feel anything different when browsing the website. ### Starters Deploy Preview vtex-sites/starter.store#334 ## References https://the-guild.dev/graphql/codegen/plugins/presets/preset-client https://the-guild.dev/blog/optimize-bundle-size-with-swc-and-graphql-codegen I read this code a bunch to figure out how client-preset worked and how to use the options: https://github.com/dotansimha/graphql-code-generator/blob/master/packages/presets/client/src/index.ts https://nextjs.org/docs/messages/swc-minify-enabled https://nextjs.org/docs/architecture/nextjs-compiler dotansimha/graphql-code-generator#9057 --------- Co-authored-by: Mariana Caetano Pereira <67270558+Mariana-Caetano@users.noreply.github.com> Co-authored-by: Fanny Chien <fanny.chien@vtex.com.br>
Which packages are impacted by your issue?
@graphql-codegen/client-preset-swc-pluginDescribe the bug
I've used
swc-plugin-graphql-codegen-client-preset-optimizer-test@0.1.4before, which works without any issues. When switching to@graphql-codegen/client-preset-swc-plugin@0.1.1, however, the following error occurs:Error Log
next.config.mjsProject structure (Monorepo)
Expected behavior
No out of bounds memory access error is raised
Platform
macOS@13.0next@13.2.0node@18.13.0graphql@16.6.0@graphql-codegen/cli@3.2.1@graphql-codegen/client-preset@2.1.0@graphql-codegen/client-preset-swc-plugin@0.1.1The text was updated successfully, but these errors were encountered: