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

Import randomly becomes empty object with Webpack esbuild-loader and CJS output format after 0.14.5 #2037

Closed
Josh-Cena opened this issue Feb 20, 2022 · 8 comments

Comments

@Josh-Cena
Copy link

Josh-Cena commented Feb 20, 2022

The Docusaurus website is using esbuild-loader to leverage esbuild's speedy transpilation while avoiding architectural changes (since we are still built on Webpack). The configuration looks like this:

(isServer) => ({
  loader: require.resolve('esbuild-loader'),
  options: {
    loader: 'tsx',
    format: isServer ? 'cjs' : undefined,
    target: isServer ? 'node12' : 'es2017',
  },
}),

The isServer is a flag distinguishing between server-side rendering and client-side rendering. This configuration has worked rather well until upgrading esbuild-loader from 2.16 to 2.18. The only (significant) change introduced in this version bump is upgrading esbuild from 0.13.15 to 0.14.23.

To reproduce, clone the Docusaurus repo, upgrade esbuild-loader in the website folder to 2.18, and run yarn build --locale en. The client build succeeds, but the server build (which is essentially running the Docusaurus app in Node through ReactDOMServer.renderToString) errors with:

Error: Minified React error #130; visit https://reactjs.org/docs/error-decoder.html?invariant=130&args[]=object&args[]= for the full message or use the non-minified dev environment for full errors and additional helpful warnings.
    at a.b.render (main:326463:32)
    at a.b.read (main:326459:83)
    at Object.exports.renderToString (main:326470:138)
    at doRender (main:138856:41)
    at async render (main:138835:12)

The Minified React error is interpreted as: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

All the files that are imported as React components are in ESM format, so there shouldn't be an interop problem. Removing the format: 'cjs' line—and thus leveraging Webpack's own ESM bundling mechanisms—solves the bug. This makes me think that it's a problem with esbuild's emit being incompatible with Webpack. Leveraging Yarn's resolutions, I could confirm that build is successful with esbuild v0.14.3, but fails with v0.14.5, which turns our focus to the adjusted handling of default and __esModule in #1849. While I admittedly don't know exactly what breaking changes that refactor brings, it claims to improve compatibility with Webpack while in reality breaks Webpack.

I would admit that Docusaurus doesn't have good source maps and these SSR errors are very hard to debug, so an easy way to investigate is forgoing the server/client rendering distinction and always emitting CJS with format: 'cjs'. Since development mode only uses client rendering, we can now get HMR as well as full error messages in development. I'm able to narrow this problem down to two specific places.

We have a line: import ErrorBoundary from '@docusaurus/core/lib/client/exports/ErrorBoundary'; (using a Webpack alias which I've expanded here). The ErrorBoundary file default-exports a class component. However, when it's imported, the result is an empty object {} instead of a class. What's interesting is that this module is imported in three distinct places:

  • App.tsx and Error.tsx, both in the @docusaurus/core/lib/client folder
  • The third file, linked above, in another package

And the App.tsx imports the class successfully, while the other two import empty objects. I've tried adding another named export export {ErrorBoundary};, but even when using console.log(require('@docusaurus/ErrorBoundary'), the result is an empty object. In the App.tsx where the import is successful, it logs the expected

{
  ErrorBoundary: class ErrorBoundary
  default: class ErrorBoundary
  __esModule: true
}

Another offending place is import NavbarItem from '@theme/NavbarItem'; which also results in an empty object {} being returned. But the default export of @theme/NavbarItem, in this case, is a normal function, and this file is also imported in multiple places and only fails in one particular file.

This error may be hard to investigate without a good understanding of Docusaurus' architecture, so I'd be more curious about how I'd debug this myself: for example, can I intercept esbuild's output? Are there compatibility flags? I'd be happy to assist any debugging efforts.

Reference: facebook/docusaurus#6235

@Josh-Cena Josh-Cena changed the title Wrong import shape with Webpack esbuild-loader and CJS output format after 0.14.5 Import becomes empty object with Webpack esbuild-loader and CJS output format after 0.14.5 Feb 20, 2022
@Josh-Cena Josh-Cena changed the title Import becomes empty object with Webpack esbuild-loader and CJS output format after 0.14.5 Import randomly becomes empty object with Webpack esbuild-loader and CJS output format after 0.14.5 Feb 20, 2022
@evanw
Copy link
Owner

evanw commented Feb 20, 2022

What are the steps to reproduce the issue? Is it possible to reproduce this by just building with esbuild without Webpack involved? I don't know anything about the esbuild-loader project myself.

@Josh-Cena
Copy link
Author

I will try reproducing this with esbuild alone, maybe in the next few days. The fact is, our internals is very tightly coupled to Webpack so it's hard to extract all the relevant parts and make it build. The quickest way to reproduce (most time-efficient for me :P) is still:

  1. Clone the repo
  2. Change esbuild-loader version to 2.18.0
  3. Run yarn install
  4. Change format to always be cjs
  5. Run yarn start in the website folder

If you clone the repo, the two errors are coming from this file and this file, respectively. Because yarn install would pre-compile them to JS, the actual files loaded are in docusaurus-theme-classic/lib-next instead of docusaurus-theme-classic/src. You can add console.logs to see what's imported.

I will try to at least create a standalone website with barebones Docusaurus code tomorrow (it's getting late here). Hope that would make inspection easier.

@evanw
Copy link
Owner

evanw commented Feb 20, 2022

I was able to build it with esbuild using this code:

require('esbuild').build({
  stdin: {
    contents: `
      import ErrorBoundary from '@docusaurus/ErrorBoundary';
      console.log('imported:', ErrorBoundary);
      console.log('required:', require('@docusaurus/ErrorBoundary'));

      import NavbarItem from '@theme/NavbarItem';
      console.log('imported:', NavbarItem);
      console.log('required:', require('@theme/NavbarItem'));
    `,
    resolveDir: __dirname,
  },
  bundle: true,
  loader: {
    '.js': 'jsx',
    '.css': 'text',
  },
  plugins: [{
    name: 'hacks',
    setup(build) {
      let mappings = {
        '@docusaurus/': '@docusaurus/core/lib/client/exports/',
        '@theme/': '@docusaurus/theme-classic/lib-next/theme/',
        '@theme/Error': '@docusaurus/core/lib/client/theme-fallback/Error',
      }
      build.onResolve({ filter: /.*/ }, async args => {
        if (args.pluginData === 'skipme') return
        for (const mapping in mappings) {
          if (!args.path.startsWith(mapping)) continue
          const result = await build.resolve(args.path.replace(mapping, mappings[mapping]),
            { importer: args.importer, resolveDir: args.resolveDir, kind: args.kind, pluginData: 'skipme' })
          if (!result.errors.length) return result
        }
      })
      build.onResolve({ filter: /^@generated\// }, args => {
        return { path: args.path, namespace: 'generated' }
      })
      build.onLoad({ filter: /.*/, namespace: 'generated' }, args => {
        return { contents: '' }
      })
    },
  }],
})

That prints this when run (with node build.js | node):

imported: [class ErrorBoundary extends Component2]
required: { default: [Getter] }
imported: [Function: NavbarItem]
required: { default: [Getter] }

I have no idea how their code base works. It doesn't appear to use normal npm path resolution rules. I just came up with some quick hacks to get it to build. In any case, the import of ErrorBoundary and of NavbarItem with esbuild appears to work fine. So maybe there's something going on with esbuild-loader.

@Josh-Cena
Copy link
Author

Josh-Cena commented Feb 20, 2022

It doesn't appear to use normal npm path resolution rules.

Indeed, we have some Webpack aliases, and your quick hack seems to get them. However, because the original error is quite random (it doesn't reproduce in every import site), I haven't found a way to reliably reproduce yet.

Esbuild-loader's code seems so simple that I can be hardly convinced that it's their bug: their loader.ts simply calls esbuild's transform with nothing more.

@evanw
Copy link
Owner

evanw commented Feb 21, 2022

Sorry but I'm not going to debug this myself. This is a huge project and the build scripts run really slowly, and I don't have the patience to debug this. The problem also only manifests when the code base is built using other tools. It seems to work fine with esbuild alone. Someone should isolate the problem down to a simple case that demonstrates the problem (ideally just with esbuild alone) to move forward on this issue.

@Josh-Cena
Copy link
Author

Yes, I'm not asking you to debug it yourself at this stage🤦‍♂️ Excuse me for sending this issue so early. I'll keep it here for reference for now while I reduce it to a minimally reproducing demo. Thanks for understanding!

@hyrious
Copy link

hyrious commented Feb 21, 2022

Although I didn't look into your package, this issue looks very like this one: #1894 (comment)

Before evan has some time to fix the behavior of esm-to-cjs, you can check the circular imports in your repo.

@Josh-Cena
Copy link
Author

Josh-Cena commented Feb 21, 2022

Yes! I had almost come to the conclusion that it's because of circular imports in these files. I'll read up that conversation.

Edit. Yup, it's the same problem. Going to close this as duplicate then. Thanks @evanw @hyrious

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

No branches or pull requests

3 participants