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

rspack support exploration #37732

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

rspack support exploration #37732

wants to merge 1 commit into from

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Mar 10, 2023

Hello!

Nearly a year ago we posted the public RFC: New Bundler in Gatsby to communicate that we're looking into alternatives to webpack (Gatsby's current bundler). After a lot of research we decided against a migration at that time and further optimized our usage of webpack since then.

As you can read in our conclusion one of the major reasons to postpone the migration were the breaking changes that would have occurred. Gatsby always used webpack so Gatsby's plugin ecosystem heavily relies on webpack APIs, meaning: If we'd change those APIs due to e.g. a completely different bundler, a lot of plugins & sites wouldn't be compatible anymore.

Thus we were very happy to be approached by Valor Software together with the Rspack team from ByteDance. Rspack boasts a high level of compatibility with webpack, directly addressing our major problem for switching bundlers. Of course, there won't be a 1:1 compatibility but our experiment so far is promising.

Important: This pull request is a proof of concept.

We're still exploring Rspack inside Gatsby and we aren't making any definitive decisions yet. However, we're happy with the results so far and want to continue exploring Rspack for Gatsby.

A special thanks to the Rspack team as they have been great to work with and helped a lot with our experiment!

Learn more about Rspack:

Technical section

We mentioned high level of compatibility, but not 1:1 level compatibility. There are certain configuration and API features that are not available. Because of that not all Gatsby features work just yet, notably Static Queries and Slices are not supported currently. More specific information will be mentioned in inline comments.

We still are using babel in this setup due to some Gatsby internal babel plugins and will continue with this approach as we work first on bundling concerns. We plan to explore swc after that.

Our chunk splitting configuration is currently disabled and Rspack default is being used right now.

In current state of this Pull Request we only use Rspack for browser bundles. Development server does support Hot Module Replacement and Fast Refresh. HTML Renderer, API Functions and Rendering Engines continue to use webpack for now.

Performance

It's too early to compare yet, we don't have feature parity yet, so we can only refer to general Rspack benchmarks

Try it out

If you are curious about this, you can check out gatsby@alpha-rspack

Co-authored-by: LekoArts <lekoarts@gmail.com>
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 10, 2023
@pieh pieh added topic: webpack/babel Webpack or babel topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Mar 10, 2023
Comment on lines +24 to +38
// actions.setWebpackConfig({
// module: {
// rules: [
// {
// test: /\.tsx?$/,
// use: ({ resourceQuery, issuer }) => [
// loaders.js({
// isPageTemplate: /async-requires/.test(issuer),
// resourceQuery,
// }),
// ],
// },
// ],
// },
// })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

module.rules[].use can't be a callback

Comment on lines +41 to +42
`$virtual/async-requires`,
`./.cache/_this_is_virtual_fs_path_/$virtual/async-requires.js`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

module.hot.accept seems to not handle alias ($virtual is an alias) and require actual module path

Comment on lines +1 to +3
.thisIsDummyRuleSoThatRspackHaveSomeInitialCSSGuaranted {
z-index: inherit;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

without actual css content we were hitting HMR problem (404 on css hot update refetch)

@@ -0,0 +1 @@
globalThis.__resourceQuery = `?hot=true`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -28,9 +28,9 @@ async function recompileDevBundle({
resolve(stats)
}
emitter.on(`COMPILATION_DONE`, finish)
webpackWatching.resume()
// webpackWatching.resume()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

resume / suspend doesn't seem available

childAssets[chunkGroup.name][rel] = childFiles
}
debugger
// for (const chunkGroup of stats.compilation.chunkGroups) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

stats.compilation.chunkGroups not available

Comment on lines +449 to +460
// include: (modulePath: string): boolean => {
// // when it's not coming from node_modules we treat it as a source file.
// if (!vendorRegex.test(modulePath)) {
// return true
// }

// // If the module uses Gatsby as a dependency
// // we want to treat it as src so we can extract queries
// return modulesThatUseGatsby.some(module =>
// modulePath.includes(module.path)
// )
// },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

module.rules[].include can't be a callback

// modulePath.includes(module.path)
// )
// },
type: opts?.isRspack ? `jsx` : `javascript/auto`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

jsx type is required to inject Fast Refresh HMR handlers

Comment on lines +462 to +463
// ({ resourceQuery, issuer }): Array<RuleSetUseItem> =>
use: [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

module.rules[].use can't be a callback

Comment on lines +642 to +643
// use,
type: `css`,
Copy link
Contributor Author

@pieh pieh Mar 10, 2023

Choose a reason for hiding this comment

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

Use builtin css support instead of css-loader, mini-css-extract-plugin etc

Comment on lines +68 to +71
plugins.define = someObject => {
rsPackDefine = { ...rsPackDefine, ...someObject }
return undefined
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -210,7 +223,7 @@ module.exports = async (

function getPlugins() {
let configPlugins = [
plugins.moment(),
// plugins.moment(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IgnorePlugin used by plugins.moment() not supported

Comment on lines +261 to +266
// configPlugins.push(
// plugins.extractText({
// filename: `[name].css`,
// chunkFilename: `[id].css`,
// })
// )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use builtin css support instead of mini-css-extract-plugin

Comment on lines +374 to +376
// descriptionData: {
// type: `module`,
// },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

descriptionData fails config validation

@@ -491,6 +504,7 @@ module.exports = async (
`@gatsbyjs/webpack-hot-middleware`
),
$virtual: getAbsolutePathForVirtualModule(`$virtual`),
webpack: getPackageRoot(`webpack`),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dedupe webpack package - gatsby has webpack dependency and now @rspack/dev-client also has webpack dependency. If package manager chose to install 2 instances, that breaks HMR. So we use alias here to "dedupe" it in compiled bundle

Comment on lines +597 to +620
// splitChunks: {
// chunks: `all`,
// cacheGroups: {
// default: false,
// defaultVendors: false,
// framework: {
// chunks: `all`,
// name: `framework`,
// test: FRAMEWORK_BUNDLES_REGEX,
// priority: 40,
// // Don't let webpack eliminate this chunk (prevents this chunk from becoming a part of the commons chunk)
// enforce: true,
// },
// // Bundle all css & lazy css into one stylesheet to make sure lazy components do not break
// // TODO make an exception for css-modules
// styles: {
// test(module) {
// return isCssModule(module)
// },
// name: `commons`,
// priority: 40,
// enforce: true,
// },
// },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

splitChunks need to be rethinkied. No function callbacks allowed here (test, name, etc)

@@ -737,21 +752,21 @@ module.exports = async (
},
// TODO fix our partial hydration manifest
mangleExports: !isPartialHydrationEnabled,
splitChunks,
// splitChunks,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

splitChunks need to be rethinkied. No function callbacks allowed here (test, name, etc)

}
}
)
// compiler.hooks.finishMake.tapPromise(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

compiler.hooks.finishMake hook not available

// )
// for (const entry of compilation.entries.values()) {
// for (const dependency of entry.dependencies) {
// const mod = compilation.moduleGraph.getModule(dependency)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

compilation.moduleGraph not available in JavaScript. It is used internally in Rust land, but not exposed to JS

@tujoworker
Copy link

tujoworker commented Mar 12, 2023

From the docs:

We are very willing to provide support to framework teams and toolchains within the community to unleash the true performance advantages of Rspack. If your framework or toolchain has a demand for high-performance build engines, let us know!

Comment on lines +239 to +246
// const incomingConnections =
// compilation.moduleGraph.getIncomingConnections(module)
// for (const connection of incomingConnections) {
// if (connection.originModule instanceof NormalModule) {
// traverseModule(connection.originModule, config, visitedModules)
// }
// }
// }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we can figure out alternative way to "backtrack" imports that is not relying on moduleGraph.getIncomingConnections?

Or if we zoom out of this implementation detail, what could be other way to determine if module A is used/needed/imported by module B?

There is additional consideration to make here:
For React Server Components support (experimental, not on by default) we have to do this moduleGraph traversal BEFORE we mess with module graph to make React Server Components work by removing non-client components and add entries for client components, so that's something to keep in mind that we likely can't just use final Stats object for that (while it probably be fine for Rspack, as it would be ok to not support Server Components there yet, this would introduce separate code path / implementation that adds too much maintainance burden to reallistically support and possibly introduce behavior changes between the 2 implemantions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: webpack/babel Webpack or babel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants