Msw: Tree shake graphql?

[kentcdodds]

If I'm not using graphql, it'd be cool if that didn't end up in my client. Do you think it's possible to make the graphql dep tree-shakeable?

Sorry, I don't think the template applies here.

[kettanaito]

Hi, @kentcdodds. No worries, my bad, there should be a freeform template for such occasions.

I'd approach this in two steps:

1. Make sure msw is bundled so it's tree-shackable.
2. Tree shake the graphql import, as the library doesn't actually use much of it (I believe it's only a parse function).

I will try to look into this in the nearest future. Thanks!

[kentcdodds]

I would probably suggest this is a low-priority feature. The only reason I care is because I actually do ship msw to production in https://bookshelf.lol which is not recommended and nobody should really do. But I am wanting to do perf metrics on the bookshelf app (for demonstration purposes) and there's a bunch of code from graphql (it's pretty huge) that's unused in my app.

So yeah, low priority, but definitely appreciated :)

[kettanaito]

After playing around with rollup, I was able to shake off ~20KB of the bundle size, and compile the library to ESM. However, graphql is imported due to the main entry exporting graphql and graphqlContext. I wasn't able to tree shake it in a test project with webpack + ESM build of the library either.

It seems that the best solution here is to bundle the graphql request handler separately, making it usage opt-in.

import { composeMocks } from 'msw'
import graphql from 'msw/graphql'

I dislike how the API becomes separated like this, but don't see a better way not to force all the users to include GraphQL-related modules when they are not using GraphQL.

[kettanaito]

As of 0.14.0, the library now distributes with the ESM module as well. @kentcdodds, could you please try to see how it behaves in your build?

Since ESM is susceptible to tree shaking, I wonder if using this format may solve the problem of unwanted imports. If not, I'd also suggest to try to configure your build to ignore /graphql/ dependencies altogether. If this could be solved on the build's size, I'd be happy.

Meanwhile I'll learn more about how to bundle the library so it's tree shakable.

[kentcdodds]

Just tried this and the bundle is just as big. Checking the minified version of the code, I do still have references to graphql.

Unfortunately my app uses create-react-app so I cannot customize the webpack config without ejecting.

[kettanaito]

@kentcdodds, thanks for trying that. I know that not everybody is in charge of their build setup, so I want MSW to take this into account.

Well, so now it's up to proper bundling of the library again. I'll take a look at how ramda works. In my experience it allows to import multiple things directly, while only those you used will remain in your end bundle.

[kentcdodds]

I don't think this applies, but it might be helpful: https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

You might be able to put the side-effect free code in a directory with a package.json that labels it as side-effect-free and get improve tree-shaking from that.

[kettanaito]

Marking modules as side-effect free is a great suggestion, thanks! From what I can gather, it works in an inverted way, meaning you mark which modules _have_ side-effects (if speaking about sideEffects option from webpack).

I've tried setting sideEffects: false in package.json, yet that yielded no result. As the next step, I've tried marking GraphQL-related modules with the /*#__PURE__*/ special comment respected by minifiers. That didn't produce any change either.

Overwhelmed by desperation, I've reached out to the Rollup chat, where @frank-dspeed provided an extensive consultation over how to manage entry points and bundle the library more efficiently. Following those suggestions, I've split the build process to create three main entries:

lib/
  - index.js // packageJson.main
  - rest.js
  - graphql.js

Obviously, removing the import statements to rest and graphql from the main entry doesn't include them in the index.js bundle. However, one still needs to be able to reference them.

When index.js has no imports to those modules, they are also not known for TypeScript.

I've also had a "positive" result when appending the module reference manually using output.footer option in Rollup:

footer: "export { graphql } from './graphql'"

That adds the line to the bundle _after_ the bundling is done. Hacky, but I saw GraphQL disappear from a demo project, while still being able to import it from index.js. Sadly, that is not the way to go, plus TypeScript support breaks this way.

I can see how having separate modules is beneficial: if imports are orchestrated correctly, one can indeed omit the entire module.

What I'm struggling right now:

1. How to reference rest and graphql from within index.js so it's not bundled to it, but preserves imports? This is crucial for API and TypeScript intellisense.
2. How to be with the UMD bundle, that cannot be split like this.

At this point I'm not sure what to do. If anybody has a minute to investigate, I've pushed the changes I have in #138.

[kentcdodds]

To be clear, your goal is to make this change without any breaking changes so people do not have to import from a msw/rest or msw/graphql module? Is that right?

I'm not certain why tree-shaking isn't ditching the graphql exports when they're unused. It feels like that should just get removed 🤔

[frank-dspeed]

https://github.com/frank-dspeed/msw/tree/110-tree-shake-graphql i have made a fork to test tree-shaking it works graphql is out of the test build in lib/tree

mkdir test-treeshaking
cd test-treeshaking
git clone https://github.com/frank-dspeed/msw --branch 110-tree-shake-graphql --single-branch .
NODE_ENV=production rollup -c rollup.config.ts
rollup -c
cat lib/tree/tree-shaked.js | grep graphql

[frank-dspeed]

ok now going to answer the question

1. is solved and demonstrated in my fork.
2. if you want to export as umd you need to only use index.ts for that and add a extra build for that
3. if you need something for the web that works use systemjs and not umd

[kettanaito]

Hey, @frank-dspeed. Thank you for the help!

I've checked out your fork and build the library, pointed the entry to lib/esm/index.js and tried to test it in the test webpack project. Unfortunately, the application's (not the library's) build is still not tree-shaken:

Usage

import { setupWorker, rest } from "msw";

const worker = setupWorker(
  rest.get("/users", (req, res, ctx) => {
    return res(
      ctx.json({
        firstName: "John",
      })
    );
  })
);

worker.start();

Bundle analyzis

There's ~80KB of graphql included in the build, although I do not import graphql from msw in the usage example.

Perhaps there's a difference depending what bundler you use. I'd orient on webpack, as it's used much more for application bundling.

My other thought would be regarding those shared utils that you've mentioned. Perhaps there is an indirect reference to graphql.js within setupWorker(), that causes that file to be considered used. I can see that in lib/esm/index.js there is a permanent usage of graphql.js to pull in data and errors functions from the context utilities:

// lib/esm/index.js
import { d as data, e as errors } from './graphql2.js';
export { g as graphql } from './graphql2.js';

var index = /*#__PURE__*/Object.freeze({
    __proto__: null,
    status: status,
    set: set,
    cookie: cookie,
    body: body,
    data: data, // <- here
    delay: delay,
    errors: errors, // <- and here
    fetch: fetch,
    json: json,
    text: text,
    xml: xml
});

Maybe because of this graphql2.js module is always considered used. Those data and errors functions are never used outside of the graphql module, so it's safe to move them into graphql.js, so the main entry stops relying on them.

Could you please elaborate more on what your fork introduces? I've spotted only:

- format: 'es'
+ format: 'esm'

[kettanaito]

@kentcdodds, yes, the point is not to introduce any breaking changes, but also to keep the current public API design. I'd find it quite confusing to require separate imports for the library's _core_ functionality (as both REST and GraphQL are primary features).

I'm sure my understanding of this issue is still limited, but I'd try to describe it below.

Bundle

I believe the reason why GraphQL is not tree-shaken as of 0.15.5 is because it's imported directly in the entry file:

// src/index.ts
export { default as graphql } from './handlers/graphql'

Rollup spots this import and puts the contents of handlers/graphql.ts into the entry file. It seems that once there, it can be tree-shaken by other tools, although build an ESM.

Splitting the entry

One of the code ideas in the current discussion is to split the main entry, so that rest.ts and graphql.ts request handlers are bundled separately. When done so, Rollup would automatically replace the graphql.ts import in the main entry, and replace it with the import to the _built graphql.js module_, relative to the build's directory.

This should allow to mark graphql.js as side-effects free, and I'd expect webpack to drop it, if it's not referenced from within the application's code. However, this doesn't happen.

[kettanaito]

When I've added an extra src/context/index.ts entry point to the Rollup config, I see that the graphql is gone from the webpack build:

I think this is the way to go. Now I've left to sort out those ugly generated chunks 😄

This is a great start. There's still some tasks to do, which I've mentioned in the pull request (#138), and once those are finished we can release the tree shaking support.

[frank-dspeed]

@kentcdodds the fork has 2 added files and produces a lib/esm/tree folder to demo that tree shaking works you do not need to merge as you already found out that it works maybe.

[kettanaito]

Is there any general recommendation on how to manage file names for those shared chunks?
I find index2.js a pretty confusing name in a stacktrace.

[frank-dspeed]

name them [name]-deps or -bundle something like that you can also call them -chunk

[kettanaito]

Thank you everybody for involvement!

As of 0.15.7 the library ships with an ESM module that is tree-shakable. If you've got a chance, I'd be thankful to confirm that.

[kentcdodds]

Sweet! That change dropped my bundle from 96.4 KB to 87.63 KB (those are post-gzip numbers). Overall, the JS file where msw is chunked into was 321 KB (not gzipped) and it's now 283 KB (not gzipped)! That's over a 10% savings for me.

Thanks for doing this work 👏

[kettanaito]

@kentcdodds, so glad to hear this task was worth the effect! Thanks once more for raising this, I believe those are good changes even for development environment.