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

fix: SWC plugin #9489

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

fix: SWC plugin #9489

wants to merge 19 commits into from

Conversation

YassinEldeeb
Copy link
Collaborator

@YassinEldeeb YassinEldeeb commented Jun 6, 2023

@changeset-bot
Copy link

changeset-bot bot commented Jun 6, 2023

⚠️ No Changeset found

Latest commit: cba43e0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@YassinEldeeb
Copy link
Collaborator Author

oops, based off of the wrong branch

@CaptainN
Copy link

Possible to get this merged? The swc plugin doesn't work at all right now.

@focux
Copy link

focux commented Jul 3, 2023

Is there any progress on this?

@saihaj
Copy link
Collaborator

saihaj commented Jul 7, 2023

@YassinEldeeb let's rebase this one now?

@CaptainN
Copy link

I recreated this on top of master, and updated to swc_core 0.79.x but was unable to produce a binary that didn't panic.

info Creating an optimized production build .thread 'thread '<unnamed><unnamed>' panicked at '' panicked at 'failed to invoke plugin: failed to invoke plugin on Some(... every file)

I don't know enough about swc or rust to go much further.

@CaptainN CaptainN mentioned this pull request Jul 25, 2023
@MatteoGiacomo
Copy link

Ehy guys, do you have any updates on this? @YassinEldeeb @saihaj, thanks

@YassinEldeeb
Copy link
Collaborator Author

YassinEldeeb commented Aug 10, 2023

SWC Plugin Concerns & Clarifications:

A while ago, I discussed with the team the SWC plugin and the challenges with its integration. Here's an overview of our findings:

1. SWC Core Versioning and Compatibility:

  • On mid-may, I discovered that SWC provided documentation on selecting the correct SWC Core version. Essentially, each minor version supports a range of nextjs versions, specifically @swc/core - the Next.js compiler.

  • An important note: Wasm plugins aren't backward compatible. Hence, selecting the correct swc_core version for your plugin is crucial.

From SWC Documentation: "Currently, the Wasm plugins are not backwards compatible. So you need to select an appropriate version of swc_core for your plugin."

LINK to the documentation

2. My Initial Approach:

  • I explored creating an installation CLI. It would read the nextjs version in a project’s package.json and determine the right SWC plugin version to use.
  • When publishing from our end, the SWC plugin would provide versions like:
    • @graphql-codegen/swc-plugin@v0.78.x
    • @graphql-codegen/swc-plugin@v0.76.x
    • @graphql-codegen/swc-plugin@v0.75.x

And the CLI would decide on the installation version based on the read nextjs version from the project. And, of course, upgrading your nextjs version would likely break the SWC plugin silently.

3. Renovate Configuration Issues:

  • Our existing renovate config, which issues PRs to regularly update the swc_core, seems obsolete. It tends to favor the latest or even canary versions of Next.js, and that's what introduced the breaking changes from the beginning until now with the SWC plugin not working unless with a specific range of Next.js versions which is specified in their documentation.
  • A potential solution could be to have it auto-publish the plugin with the upcoming SWC core version. However, users must be aware of the need to pin the swc_core version. If they update their Next.js version, the SWC plugin also needs an update. The error messages, as you've all seen, might not be straightforward.

4. Future Challenges:

  • Keeping up with the frequent releases of SWC Core and Next.js versions is challenging. One idea might be to automate some build and publishing steps based on scraping their documentation, but there's no guarantee it'll always be updated promptly.

5. Development Concerns for SWC Plugin:

  • Currently, swc_core is developed on the Rust nightly version, and not the stable toolchain. We've had to adapt to that environment.

Hoping this sheds some light on the SWC plugin challenges. Feedback and suggestions are welcome!
And I apologize for the long wait on this.

We've been busy preparing for the first GraphQL Conf 2023 held in San Francisco, and we plan to connect with SWC maintainers and figure out a solution together once we get back. I appreciate your patience! 🙏

6. Okay, but what for now?

As for now, it appears to be that SWC plugins are meant to be built and used at an individual level, so you can:

  1. Clone the SWC plugin https://github.com/dotansimha/graphql-code-generator/tree/master/packages/presets/swc-plugin
  2. Change swc_core version in Cargo.toml matching your next.js version, please look up how to find your suitable swc_core version here https://swc.rs/docs/plugin/selecting-swc-core
  3. Follow the toolchain installation guide from here https://swc.rs/docs/plugin/ecmascript/getting-started#install-required-toolchain
  4. Run npm run build-wasm to compile it
  5. This would result in a swc_plugin.wasm file which is linked to the SWC plugin package.json via the "main": "swc_plugin.wasm"
  6. Use your local package within your project via npm link or workspaces implementation of the various package managers (npm, yarn, or pnpm)
  7. And, it works! 🎉

@victorandree
Copy link

@YassinEldeeb I appreciate the update! I tried to follow your steps on how to update this on my own – but it fails in different ways (basically confirming what @CaptainN commented above).

Leaving this comment so maybe the community can figure this out.

  • If I specify swc_core@0.79.*, which should work with next@v13.4.10-canary.1 ~ (according to https://swc.rs/docs/plugin/selecting-swc-core#v079x), I can build the plugin. However, if I try to then use this plugin with next@13.4.13 (latest, as of today) or next@13.4.10-canary.1, I get errors about "out of bounds memory access" (full stack trace below).
  • Note: If I just use the current plugin with next@13.4.13, the error is instead RuntimeError: unreachable.
  • If I simply try to build the current version of the swc-plugin (which specifies swc_core@0.75.*), I get a build error from proc-macro2 about "unknown feature proc_macro_span_shrink". I'm guessing this is because we have to build this with the Rust nightly toolchain, as mentioned in the comment by @YassinEldeeb, and something has changed here.

Stack trace from swc_core@0.79.* with next@13.4.13. Node version is 18.17.1 (latest LTS).

Error: failed to process failed to invoke plugin: failed to invoke plugin on 'Some("/c/next-app/pages/index.tsx")'

Caused by:
    0: failed to invoke `/c/next-app/node_modules/@graphql-codegen/client-preset-swc-plugin/swc_plugin.wasm` as js transform plugin at /c/next-app/node_modules/@graphql-codegen/client-preset-swc-plugin/swc_plugin.wasm
    1: RuntimeError: out of bounds memory access

Stack backtrace:
   0: _napi_register_module_v1
   1: _napi_register_module_v1
   2: _BrotliDecoderVersion
   3: _BrotliDecoderVersion
   4: _BrotliDecoderVersion
   5: _BrotliDecoderVersion
   6: _BrotliDecoderVersion
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: <unknown>
  16: <unknown>
  17: <unknown>

Stack backtrace:
   0: _napi_register_module_v1
   1: _napi_register_module_v1
   2: <unknown>
   3: <unknown>

Import trace for requested module:
./pages/index.tsx

@victorandree
Copy link

victorandree commented Aug 11, 2023

Hopefully relevant: The build failure is because of swc-project/swc#7479. Setting lto = false for the release profile allows me to build and use a new version of the plugin 🎉.

I've created a fork and published an npm package that changes lto = false, updates swc_core, and fixes #9445. Looking to get these updates merged upstream - but others might find it useful. Absolutely no support will be provided from me 🙃.

Update Sep 28: I've updated my fork and library to work with Next.js 13.5.3. Approach is basically to update swc_core to the latest version. Note that I rebased my master branch on upstream, which broke previous commits.

@JamesCodes
Copy link

JamesCodes commented Aug 18, 2023

Hopefully relevant: The build failure is because of swc-project/swc#7479. Setting lto = false for the release profile allows me to build and use a new version of the plugin 🎉.

Thank you! This saved me.

@YassinEldeeb
Copy link
Collaborator Author

That's absolutely amazing, @victorandree!! Sorry, I was late getting back to you,
I'm super glad you were able to figure out the issue! ❤️

@bvobart
Copy link

bvobart commented Sep 11, 2023

Hi @YassinEldeeb and @victorandree, thank you both for this MR and your patched NPM package!

This has fixed the original syntax error that I was getting in #9337. Without any further updates to the repo, I was getting an out of bounds memory access error like in #9489 (comment), but that was fixed by upgrading @swc/core to 1.3.80 as prescribed by the SWC docs.

However, the issue isn't entirely fixed yet. As soon as I import code from another TypeScript file, I get a different syntax error, even though I've updated my example repo for #9337 to use the patched SWC plugin and I've updated all dependencies to versions that should be compatible (in particular @swc/core).

Could you look into this again please? 😊

@victorandree
Copy link

FWIW, I recently noticed that my Next.js builds started ballooning in size, so I wanted to have another look at this issue (suspecting that maybe the plugin broke again). While I haven't had any issues using the plugin with Next.js 14.0.2, I figured it's good to be up-to-date.

TL;DR: New version of my fork released as @victorandree/graphql-codegen-client-preset-swc-plugin@0.3.0, using swc_core@0.86. Updating swc_core required me to change test code; see my fork at https://github.com/victorandree/graphql-code-generator. Unfortunately, did not help with bundle size.

  • @YassinEldeeb Now that GraphQL Conf 2023 is over, have you had the chance to talk to the SWC maintainers about a way forward?
  • @bvobart Unfortunately, I'm not familiar with using @swc/jest to transform files for jest. I tried running your reproduction repository, and for me it fails even if I remove the plugin. At a glance, it seems related to your TypeScript code not being compiled - because the syntax error is related to a: number in Foo.ts.

IMHO, the concern about matching swc_core and Next.js versions is valid, but we're also all consenting adults here, who are using the known unstable swcPlugins feature and are seeing "Experiments (use at your own risk): swcPlugins" on every build with a big warning about it in the docs.

For now, I think it'd be worthwhile to release a new version of the plugin with an updated swc_core, considering it's been broken at a patch version of Next.js for some time now.

@YassinEldeeb
Copy link
Collaborator Author

Hey, really sorry for the very late reply! We are currently tracking the issue with SWC maintainers to figure out a solution.

Please add a 👍 to the issue and feel free to join the conversation.
swc-project/swc#8315

I'll keep you updated!

@YassinEldeeb
Copy link
Collaborator Author

@victorandree @bvobart @MatteoGiacomo @CaptainN @focux as you can see, unfortunately, they replied simply with In short, this is impossible, please refer to the above issue's comment from the maintainers.

@victorandree
Copy link

@YassinEldeeb I appreciate trying to sync with the SWC maintainers on this, and I also appreciate the challenges of trying to keep things in sync between SWC/Next.js versions (and that Next.js isn't the only game in town).

That said, would you consider releasing new versions of the plugin with documentation to clarify compatibility, or is this effectively unmaintained until SWC stabilizes?

@YassinEldeeb
Copy link
Collaborator Author

YassinEldeeb commented Nov 23, 2023

@victorandree I really appreciate your enthusiasm and engagement in this issue!

would you consider releasing new versions of the plugin with documentation to clarify compatibility

Unfortunately, SWC maintainers weren't able to promise us with up to date documentation on the compatibility between next.js versions and rust's swc_core crate (package), which is the main concern causing all of the issues we're all currently facing.

is this effectively unmaintained until SWC stabilizes?

It's difficult, but I wouldn't say it's completely unmaintained, it's just about chasing up to date compatibility information from SWC, they have effectively claimed plugins to be a "Beta Feature", to avoid people complaining about them releasing breaking changes without notices. So it's tricky.

We are thinking of moving the SWC plugin to the GraphQL Codegen Community repo, where the community can file issues of breaking changes as they detect it, lookup https://swc.rs/docs/plugin/selecting-swc-core for updated documentation, if not, ask SWC maintainer to update it by filing an issue, then finally releasing a new version of the SWC plugin, so there would be:

  • @graphql-codegen/swc-plugin@v0.78.x
  • @graphql-codegen/swc-plugin@v0.76.x
  • @graphql-codegen/swc-plugin@v0.75.x

Where each new version doesn't override the previous one, but adds support for newer next.js versions.

So, what do you think @victorandree? Would it be feasible to let the community (including most people in the current thread) take care of maintaining it given the challenges mentioned?

We think if the community can quickly detect breaking changes, and one person can fix it and release it for everyone else, then it can work.

Curious to hear your thoughts! 👀

@victorandree
Copy link

@YassinEldeeb

We are thinking of moving the SWC plugin to the GraphQL Codegen Community repo, where the community can file issues of breaking changes as they detect it, lookup https://swc.rs/docs/plugin/selecting-swc-core for updated documentation, if not, ask SWC maintainer to update it by filing an issue, then finally releasing a new version of the SWC plugin, so there would be:

  • @graphql-codegen/swc-plugin@v0.78.x
  • @graphql-codegen/swc-plugin@v0.76.x
  • @graphql-codegen/swc-plugin@v0.75.x

Where each new version doesn't override the previous one, but adds support for newer next.js versions.

So, what do you think @victorandree? Would it be feasible to let the community (including most people in the current thread) take care of maintaining it given the challenges mentioned?

  • I definitely hope that the community can help with maintaining this, although I'm not sure what difference it makes if this is part of dotansimha/graphql-code-generator-community or dotansimha/graphql-code-generator. The contribution guidelines seem identical for both. Maybe I'm just not up-to-date with your governance structure :)
  • I'm not super convinced on tracking SWC core versions with version identifiers in this plugin. What if you want to fix a bug or add a feature (for example, @graphql-codegen/client-preset-swc-plugin adds imports before "use client" #9445)? If you do want to maintain support for different SWC core versions, I think it'd be better and just as much of a mess to build and include multiple versions of swc_plugin.wasm in each release of the plugin (so you'd have e.g. swc_plugin-0.78.wasm, swc_plugin-0.76.wasm for however many versions you'd decide to support).
  • This would move version select to where you configure SWC plugins, instead of package.json. Next.js allows you to specify "an npm module package name or an absolute path to the .wasm binary itself" (source: https://nextjs.org/docs/architecture/nextjs-compiler#swc-plugins-experimental), so I'm admittedly not sure if this is ergonomic at all.
  • My personal opinion is that you don't need to support anything but the latest version of SWC core. This is an experimental feature, so I don't think anyone can expect backwards compatibility.

@WonderPanda
Copy link

Just discovered this thread after trying to update a project to use all the latest and greatest with the client preset. I'm very excited about the direction things are going, especially with the focus on composing fragments up the tree!

I'm not using NextJS, just a react app using Vite in an NX monorepo but I'm running into issues with the plugin. Is there a list of known compatible versions of the plugin + swc core version that I could try and pin to?

@CaptainN
Copy link

CaptainN commented Dec 4, 2023

This is an experimental feature, so I don't think anyone can expect backwards compatibility.

When you use the generator, it outputs this at the top of your file:


/**
 * Map of all GraphQL operations in the project.
 *
 * This map has several performance disadvantages:
 * 1. It is not tree-shakeable, so it will include all operations in the project.
 * 2. It is not minifiable, so the string of a GraphQL query will be multiple times inside the bundle.
 * 3. It does not support dead code elimination, so it will add unused operations.
 *
 * Therefore it is highly recommended to use the babel or swc plugin for production.
 */

That makes it sound more necessary than experimental.

@victorandree
Copy link

@CaptainN To be fair, there should also be a (stable) Babel Plugin that achieves the same results, see https://the-guild.dev/graphql/codegen/plugins/presets/preset-client#reducing-bundle-size. This is about the SWC plugin specifically.

@CaptainN
Copy link

CaptainN commented Feb 6, 2024

@victorandree That's true, but it's pretty challenging to get that to work with nextjs, which uses SWC by default. (It has a way to change back to babel, but it breaks a bunch of stuff - not easy to pull off.)

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.

None yet

10 participants