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

ESM support + treeshaking unused bits #1384

Closed
1 task
mattcosta7 opened this issue Sep 1, 2022 · 16 comments · Fixed by #1436
Closed
1 task

ESM support + treeshaking unused bits #1384

mattcosta7 opened this issue Sep 1, 2022 · 16 comments · Fixed by #1436
Labels

Comments

@mattcosta7
Copy link
Contributor

Scope

Improves an existing behavior

Compatibility

  • This is a breaking change

Feature description

In #1356 we tried (and failed) to make graphql a dependency that wasn't required for users who don't actually do any graphql mocking.

We should work to make that something we can support going forward though, as well as to generally make the msw package available as an esmodule build to better support treeshaking of unused code (like potentially graphql)

@kettanaito
Copy link
Member

Hey, @mattcosta7. Absolutely agree with this effort. We need to have proper ESM support and ensure tree-shaking. With all the incredible contributions that you're shipping recently, would you like to champion this one?

@mattcosta7
Copy link
Contributor Author

I have some ideas for how this might work. Landing a version of #1383 would be a good first step.

i think a path forward from there would be

create a transpiled only (but not bundled) build

potentially like

// pseudo config - untested
  {
    name: 'module',
    entry: ['./src/index.ts'],
    outDir: './lib',
    format: ['esm'],
    legacyOutput: true,
    sourcemap: true,
    clean: true,
    bundle: false,
    splitting: true,
    dts: false,
    esbuildPlugins: [workerScriptPlugin()],
  }

From there we can at least offload the treeshaking/bundling to the consumer build tooling more effectively

At that point, analyzing what can be treekshaken will be helpful. I don't believe many tools effectively treeshake 're-exports', so we'll likely still incur the parse function from graphql being pulled into consuming bundles - but likely nothing else from graphql.

At that point we should be able to evaluate whether to expose lib/esm/graphql directly as msw/graphql, removing the direct re-export, which should allow code from these to be shared by consumers but also avoid including the graphql code at all

note - this is probably not a big deal, since msw is a development tool first and foremost, but for some cases it might be helpful there too

@mattcosta7
Copy link
Contributor Author

Hey, @mattcosta7. Absolutely agree with this effort. We need to have proper ESM support and ensure tree-shaking. With all the incredible contributions that you're shipping recently, would you like to champion this one?

I should be able to 👍

@mattcosta7
Copy link
Contributor Author

mattcosta7 commented Sep 1, 2022

This is just a diff off a local copy for now, until I can actually prove some of this work out a bit

https://github.com/mattcosta7/msw/pull/1/files

some first pass thoughts on what that might look like changeset wise.

@mattcosta7
Copy link
Contributor Author

mattcosta7 commented Sep 1, 2022

I think we'll need to also publish interceptors and cookies as esm builds to work with an esm build of msw in browsers directly

@kettanaito
Copy link
Member

@mattcosta7, that's one of my concerns as well. ESM is like async/await: you introduce it in one place and now you need to introduce it everywhere. We need to take our dependencies into account and make sure we also ship ESM support in interceptors, headers, cookies—just like you said. We can use TSUP for that.

I'm taking some time off from open source but I'm active in pull requests so I always have some time to provide code review and guidance.

@kettanaito
Copy link
Member

For "proving it works" we have in-browser tests for module compatibility:

https://github.com/mswjs/msw/tree/2d9f3b4122ff00b7e2b9c28248d4f8597c240b6d/test/msw-api/distribution

You can add a new esm.test.ts and see the existing tests for inspiration. The idea behind such tests is the following:

  1. You build the library into a bundle format (i.e. ESM).
  2. You create an in-browser test that loads that bundle format.
  3. You assert that there are no runtime errors and the library has activated successfully.

There used to be a test for ESM but then I realized it wasn't correct so I've removed it.

@mattcosta7
Copy link
Contributor Author

mattcosta7 commented Sep 10, 2022

Starting to make a little progress here. A bit slower than I had hoped (but not slower than expected)

First step - getting msw (and friends) loading in a browser - using web/dev-server with node-resolved in a type=module script

Screen Shot 2022-09-10 at 10 29 55 AM

Screen Shot 2022-09-10 at 10 31 08 AM

starting with this config for web/dev-server

will try to minimize this a bit before moving on to figuring out how best to start pushing code to various repos (ordering and versioning/etc)

import { rollupAdapter } from '@web/dev-server-rollup';
import nodePolyfills from 'rollup-plugin-node-polyfills';
import rollupReplace from '@rollup/plugin-replace';

export default {
    watch: true,
    nodeResolve: true,

    plugins:[
        // EventEmitter from `events` needs browser polyfilling
        rollupAdapter(nodePolyfills()),
        // graphql/graphql-js ships some process.env.NODE_ENV code that we end up needing
        rollupAdapter(rollupReplace({
            'process.env.NODE_ENV': '"development"',
            preventAssignment: true
        })),
    ],
};

Also found an interesting thing related to tsup.

devDependencies used in production code get bundled to ship
dependencies are all treated as external

that should allow us to start packaging some of the non-esm dependencies in a way where they'll work in esm environments, without needing to push versions (with the caveat that they'll be bundled so won't get updates - should be ok for many uses)

@mattcosta7
Copy link
Contributor Author

A general set of steps that I think we'll need to do - probably not complete

We might want to transpile and not bundle all libraries (including msw), which might provide value, but might better happen later

  1. publish @mwsjs/cookies in an esm compat form
    • This might require inlining set-cookie-parser instead of externalizing it
  2. publish header-polyfill in an esm compat way
  3. publish strict-event-emitter in an esm compat way
    • may require inlining of events, but that seems a bit eh
  4. publish interceptors in an esm compat way
    • use the esm packages above
    • may want to rethink using debug since that doesn't use esm, and bundling means we'll bundle the node build and polyfills into the browser, which we probably don't want to do.
    • We could split the imports here to browser vs node imports and try to bundle differently
  5. consume esm versions of each in msw and also provide an esm output for all exports

@mattcosta7
Copy link
Contributor Author

mattcosta7 commented Sep 14, 2022

@mattcosta7
Copy link
Contributor Author

mattcosta7 commented Oct 8, 2022

Opened a few more bits towards getting full esm support

These are relied on by mswjs/interceptors and msw

@divStar
Copy link

divStar commented Nov 4, 2022

Love the progress on this! I'd be available to test stuff if need be (I'd checkout and build locally if necessary).

@koddsson
Copy link
Contributor

koddsson commented Feb 2, 2023

@mattcosta7; Anything we can do to push this forward?

@mattcosta7
Copy link
Contributor Author

hey @koddsson!

has to land prior to msw itself being able to ship an esm compat build. Once that lands, I believe we should have a more clear path to pushing this forward

@jaschaio
Copy link

jaschaio commented Mar 9, 2023

@mattcosta7 mswjs/interceptors#313 was merged 3 weeks ago :)

Anything I can do or help with to get ESM support for MSW? Not sure where to start but happy to take a stab.

@kettanaito
Copy link
Member

Released: v2.0.0 🎉

This has been released in v2.0.0!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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

Successfully merging a pull request may close this issue.

5 participants