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

Turbopack support for Blitz #4314

Merged
merged 24 commits into from Apr 5, 2024

Conversation

timneutkens
Copy link
Contributor

@timneutkens timneutkens commented Mar 16, 2024

This PR includes the changes required to make the Blitz loaders work with Turbopack. The biggest challenge is that Blitz currently uses private fields, namely _compiler (anything _ prefixed in webpack is private) and _compiler is not available in Turbopack. Even if we were to mock the _compiler being there Turbopack does not have the concept of multiple compilers running so the name can't be relied upon to decide when to compile or not.

Potentially a better solution for the webpack implementation is to use the this.target property (also not available in Turbopack) for the webpack implementation, that way you don't have to rely on internals that could change in the future.
Even better would be passing an option for when to apply, e.g. using isServer in the custom webpack config that is applied to Next.js.

Now for the Turbopack side of things:

  • We've landed support for conditionally running loaders on e.g. browser or node, PR: add conditions for webpack loader rules vercel/next.js#63333
    • This is available on the latest canary: 14.2.0-canary.25, which I've published for you earlier this evening to try the changes in this PR.
    • There's a small configuration checking warning that we still have to resolve so it will show warnings about config being incorrect but the config I've shared below is the correct one.
  • I've refactored the Blitz loader to just always run the transform when there is no name, instead we leverage the loader configuration to correctly apply at which layer we want the loader to run
  • I've changed the require()/import() path to be relative instead of absolute since Turbopack does not support reading absolute paths since they cause uncacheable output as the path is specific to your machine.

Here's the next.config.js edits I've made to run the Blitz rpc loaders:

Notably Turbopack's glob matcher doesn't support [ and ] yet and you can't reliably escape them, we'll work on fixing that, but for now I just used * before and after the ...blitz path to wildcard match [[ and ]].

const { withBlitz } = require("@blitzjs/next")

const loaderClient = require.resolve("@blitzjs/rpc/dist/loader-client.cjs")
const loaderServer = require.resolve("@blitzjs/rpc/dist/loader-server.cjs")
const loaderServerResolvers = require.resolve("@blitzjs/rpc/dist/loader-server-resolvers.cjs")

console.log("loaderClient", loaderClient)
console.log("loaderServer", loaderServer)
console.log("loaderServerResolvers", loaderServerResolvers)

/** @type {import('next').NextConfig} */
const nextConfig = {
  experimental: {
    turbo: {
      resolveAlias: {
        "cross-spawn": "./turbopack/empty.js",
        "npm-which": "./turbopack/empty.js",
        fs: { browser: "./turbopack/empty.js" },
      },
      rules: {
        "**/*...blitz*.{jsx,tsx,js,ts}": {
          default: {
            loaders: [{ loader: loaderServer, options: {} }],
            as: "*.ts",
          },
        },
        "**/{queries,mutations}/**": {
          browser: {
            loaders: [
              {
                loader: loaderClient,
                options: {},
              },
            ],
            as: "*.ts",
          },
          default: {
            loaders: [
              {
                loader: loaderServerResolvers,
                options: {},
              },
            ],
            as: "*.ts",
          },
        },
      },
    },
  },
}

module.exports = withBlitz(nextConfig)

Please give it a try and let me know if it's working as intended, as far as I can tell it's applying the loaders correctly now but I'm not familiar enough with Blitz to know for certain that e.g. edge cases are working correctly too 🙏

Closes: ?

What are the changes and their implications?

Feature Checklist

  • Changeset added (run pnpm changeset in the root directory)
  • Integration test added (see test docs if needed)

Ensures the root context is read from the public API that webpack exposes. This is the first step for Turbopack support as Turbopack includes `this.rootContext` as well
Copy link

changeset-bot bot commented Mar 16, 2024

🦋 Changeset detected

Latest commit: 263f23d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
blitz Minor
@blitzjs/next Major
@blitzjs/rpc Major
next-blitz-auth Patch
@blitzjs/auth Major
@blitzjs/codemod Major
@blitzjs/config Major
@blitzjs/generator Major

Not sure what this means? Click here to learn what changesets are.

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

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
@siddhsuresh siddhsuresh self-requested a review March 17, 2024 15:43
@flybayer
Copy link
Member

flybayer commented Mar 19, 2024

Remaining for us to do

  • auto add turbo config in withBlitz
  • add test for turbo mode

@gengjiawen
Copy link
Contributor

@timneutkens currently a blitzjs project build consume too much memory, can't even setup build on a 2gb machine (about 3.5g from my bizzare observation), will turbo improve this case ?

@cfragkos
Copy link

@timneutkens currently a blitzjs project build consume too much memory, can't even setup build on a 2gb machine (about 3.5g from my bizzare observation), will turbo improve this case ?

No, turbopack, at the moment is for the dev server. Building will remain the same.

@gengjiawen
Copy link
Contributor

@timneutkens currently a blitzjs project build consume too much memory, can't even setup build on a 2gb machine (about 3.5g from my bizzare observation), will turbo improve this case ?

No, turbopack, at the moment is for the dev server. Building will remain the same.

But I guess it's very near, since turbo is almost 100% tests passed.

Copy link
Contributor

kodiakhq bot commented Apr 2, 2024

This PR currently has a merge conflict. Please resolve this and then re-add the 0 - <(^_^)> - merge it! ✌️ label.

@kodiakhq kodiakhq bot removed the 0 - <(^_^)> - merge it! ✌️ Kodiak automerge label Apr 2, 2024
@kodiakhq kodiakhq bot deleted the branch blitz-js:main April 2, 2024 14:34
@kodiakhq kodiakhq bot closed this Apr 2, 2024
@blitzjs-bot blitzjs-bot added this to Done in Toolkit Apr 2, 2024
@siddhsuresh siddhsuresh reopened this Apr 2, 2024
Toolkit automation moved this from Done to In progress Apr 2, 2024
@siddhsuresh siddhsuresh changed the base branch from siddharth/fix-DYNAMIC_SERVER_USAGE-errors-thrown to main April 2, 2024 14:36
@siddhsuresh siddhsuresh removed the 0 - <(^_^)> - merge it! ✌️ Kodiak automerge label Apr 2, 2024
@flybayer
Copy link
Member

flybayer commented Apr 2, 2024

!preview

@flybayer
Copy link
Member

flybayer commented Apr 2, 2024

!preview turbopack

@@ -27,7 +27,7 @@
"@blitzjs/rpc": "latest",
"@prisma/client": "4.6.1",
"blitz": "latest",
"next": "13.4.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

This better stays on next 13. since next14 broken on vercel deploy.

@siddhsuresh
Copy link
Member

!preview turbopack

1 similar comment
@siddhsuresh
Copy link
Member

!preview turbopack

Copy link
Contributor

github-actions bot commented Apr 3, 2024

Snapshot Release turbopack

@flybayer flybayer merged commit ee7bf87 into blitz-js:main Apr 5, 2024
29 checks passed
Toolkit automation moved this from In progress to Done Apr 5, 2024
@blitzjs-bot
Copy link
Contributor

Added @timneutkens contributions for doc, code and test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants