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

Code splitting is more broken with esbuild 0.10.1 #1081

Closed
iamakulov opened this issue Mar 29, 2021 · 10 comments · Fixed by #1110
Closed

Code splitting is more broken with esbuild 0.10.1 #1081

iamakulov opened this issue Mar 29, 2021 · 10 comments · Fixed by #1110

Comments

@iamakulov
Copy link

iamakulov commented Mar 29, 2021

Likely related to #399.

This is JFYI. I remember that code splitting is not fully supported, but with 0.11.0 0.10.1, it seems to be even more broken. On the Framer codebase, it generates

a) cyclic dependencies between chunks that lead to undefined is not a function errors

Example

If I upgrade to esbuild 0.11.0 and try to run Framer, I’ll get the following error:

Screen Shot 2021-03-29 at 16 52 05 1

The error happens because, in chunk-PLL4RMKF.js, Sentry tries to call __extends from tslib, and __extends is undefined:

Screen Shot 2021-03-29 at 16 52 35

But why is it undefined? If we search by __extends, we’ll find that it’s imported from another chunk called chunk-EZ2R6UVJ.js:

Screen Shot 2021-03-29 at 16 55 35

That chunk exports __extends absolutely correctly:

Screen Shot 2021-03-29 at 16 56 19 Screen Shot 2021-03-29 at 16 56 24

but what it also does is it imports a few symbols from the first chunk (chunk-PLL4RMKF.js):

Screen Shot 2021-03-29 at 16 58 28

So, ultimately, the following happens:

  • some code imports chunk-EZ2R6UVJ.js
  • that chunk imports chunk-PLL4RMKF.js
  • chunk-PLL4RMKF.js initializes and tries to call __extends from chunk-EZ2R6UVJ.js
  • because chunk-EZ2R6UVJ.js haven’t initialized yet, __extends is undefined.

Here’s the contents of chunk chunk-EZ2R6UVJ.js:

// ../../../node_modules/tslib/tslib.js
// ../../../node_modules/tslib/modules/index.js
// ../../shared/src/assert.ts
// ../../shared/src/warnOnce.ts
// ../../shared/src/deprecation.ts
// ../../shared/src/errors.ts
// ../../shared/src/lazy.ts
// ../../shared/src/logger.ts
// ../../shared/src/ServiceMap.ts
// ../../shared/src/moduleIdentifiers.ts
// ../../shared/src/ResolvablePromise.ts
// ../../services/src/runtime/typescript/ServiceChannel.ts
// ../../services/src/runtime/typescript/ServiceDebugging.ts
// ../../services/src/runtime/typescript/streams/private.ts
// ../../services/src/runtime/typescript/private.ts
// ../../services/src/runtime/typescript/ServiceErrors.ts
// ../../services/src/runtime/typescript/environment.ts
// ../../services/src/runtime/typescript/ServiceManager.ts
// ../../services/src/runtime/typescript/streams/ServiceEventEmitter.ts
// ../../services/src/runtime/typescript/channels/LocalChannel.ts
// ../../services/src/runtime/typescript/channels/MessagePortChannel.ts
// ../../services/src/runtime/typescript/channels/PostMessageChannel.ts
// ../../services/src/runtime/typescript/channels/SocketClientChannel.ts
// ../../services/src/generated/typescript/index.ts
// src/environment/index.ts

Here’s the contents of chunk chunk-PLL4RMKF.js:

// ../../../node_modules/@sentry/utils/esm/is.js
// ../../../node_modules/@sentry/utils/esm/browser.js
// ../../../node_modules/@sentry/utils/esm/polyfill.js
// ../../../node_modules/@sentry/utils/esm/error.js
// ../../../node_modules/@sentry/utils/esm/dsn.js
// ../../../node_modules/@sentry/utils/esm/node.js
// ../../../node_modules/@sentry/utils/esm/string.js
// ../../../node_modules/@sentry/utils/esm/misc.js
// ../../../node_modules/@sentry/utils/esm/logger.js
// ../../../node_modules/@sentry/utils/esm/memo.js
// ../../../node_modules/@sentry/utils/esm/stacktrace.js
// ../../../node_modules/@sentry/utils/esm/object.js
// ../../../node_modules/@sentry/utils/esm/supports.js
// ../../../node_modules/@sentry/utils/esm/instrument.js
// ../../../node_modules/@sentry/utils/esm/path.js
// ../../../node_modules/@sentry/utils/esm/syncpromise.js
// ../../../node_modules/@sentry/utils/esm/promisebuffer.js
// ../../../node_modules/@sentry/utils/esm/time.js
// ../../../node_modules/@sentry/hub/esm/scope.js
// ../../../node_modules/@sentry/types/esm/session.js
// ../../../node_modules/@sentry/types/esm/severity.js
// ../../../node_modules/@sentry/types/esm/status.js
// ../../../node_modules/@sentry/hub/esm/session.js
// ../../../node_modules/@sentry/hub/esm/hub.js
// ../../../node_modules/@sentry/minimal/esm/index.js
// ../../../node_modules/@sentry/core/esm/api.js
// ../../../node_modules/@sentry/core/esm/integration.js
// ../../../node_modules/@sentry/core/esm/baseclient.js
// ../../../node_modules/@sentry/core/esm/transports/noop.js
// ../../../node_modules/@sentry/core/esm/basebackend.js
// ../../../node_modules/@sentry/core/esm/request.js
// ../../../node_modules/@sentry/core/esm/sdk.js
// ../../../node_modules/@sentry/core/esm/version.js
// ../../../node_modules/@sentry/core/esm/integrations/index.js
// ../../../node_modules/@sentry/core/esm/integrations/functiontostring.js
// ../../../node_modules/@sentry/core/esm/integrations/inboundfilters.js
// ../../../node_modules/@sentry/browser/esm/tracekit.js
// ../../../node_modules/@sentry/browser/esm/parsers.js
// ../../../node_modules/@sentry/browser/esm/eventbuilder.js
// ../../../node_modules/@sentry/browser/esm/transports/base.js
// ../../../node_modules/@sentry/browser/esm/transports/fetch.js
// ../../../node_modules/@sentry/browser/esm/transports/xhr.js
// ../../../node_modules/@sentry/browser/esm/backend.js
// ../../../node_modules/@sentry/browser/esm/helpers.js
// ../../../node_modules/@sentry/browser/esm/integrations/globalhandlers.js
// ../../../node_modules/@sentry/browser/esm/integrations/trycatch.js
// ../../../node_modules/@sentry/browser/esm/integrations/breadcrumbs.js
// ../../../node_modules/@sentry/browser/esm/integrations/linkederrors.js
// ../../../node_modules/@sentry/browser/esm/integrations/useragent.js
// ../../../node_modules/@sentry/browser/esm/client.js
// ../../../node_modules/@sentry/browser/esm/sdk.js
// ../../../node_modules/@sentry/integrations/esm/rewriteframes.js

In chunk-EZ2R6UVJ.js, src/environment/index.ts depends on Sentry from chunk-PLL4RMKF.js. In chunk-PLL4RMKF.js, Sentry depends on tslib from chunk-EZ2R6UVJ.js.

If src/environment/index.ts was bundled separately from other modules in chunk-EZ2R6UVJ.js, this issue wouldn’t happen.

b) dependencies between chunks and other entrypoints

This one feels very incorrect – chunks should never import other entrypoints.

Example

If I work around the following error (by tuning/reshuffling a few imports), I’ll get a different error:

Screen Shot 2021-03-29 at 17 10 36

If I set a breakpoint inside that React function, I’ll see that the function is called from projects.debug.js:

Screen Shot 2021-03-29 at 17 11 10

This is super weird. projects.debug.js is an entrypoint, and it’s a different entrypoint. This file shouldn’t be loaded on this page at all.

Then why is it loaded? This is why:

Screen Shot 2021-03-29 at 17 14 28

(format is a function from date-fns/esm/format/index.js)

This feels very incorrect. Entrypoints include top-level code with concrete side effects. That code should be called only when it’s supposed to be called.

In our case, projects.debug.js calls ReactDOM.render(..., document.querySelector('#root')) – and #root doesn’t exist on the page because it’s not the right page!

Sorry for no repro! I’m not working with Framer this week so this is the best I can come up with in a limited time. I’ll be able to invest more time in a proper repro in a couple weeks if it’s needed.

@Valexr

This comment has been minimized.

@iamakulov iamakulov changed the title Code splitting is more broken with esbuild 0.11.0 Code splitting is more broken with esbuild 0.10.1 Mar 29, 2021
@iamakulov
Copy link
Author

iamakulov commented Mar 29, 2021

Wait, sorry – this has actually started happening earlier, with 0.10.1. 0.10.0 works correctly.

(I reproduced the first issue with 0.10.1. The second one is likely present there as well, but I haven’t had a chance to check yet.)

@evanw
Copy link
Owner

evanw commented Mar 29, 2021

I'm guessing this is the removal of the file splitting optimization, which is sort of a fundamental shift to how the bundler works. It's a bummer that I won't have a way to reproduce this. Unfortunately I don't think I'll be able to fix this without one. I've looked over that code a few times now and nothing stands out. And it doesn't seem to be an issue for any of the projects in my test set. I'll keep trying.

@iamakulov
Copy link
Author

Got it, thank you for checking this on your side! Added this to my to-do list; if all goes as planned, will get back with a repro later this month.

@zandaqo
Copy link

zandaqo commented Apr 3, 2021

I'm having the same issue and no luck making a minimum reproduction. I suspect in my case it has to do with chunks depending on entry points since I have a bunch of libraries loading their own versions of tslib.

Made a reproducation here: https:/zandaqo/esbuild_chunk
It has two entry points application and intro that share a common chunk, if you run the build (npm run build) you'll notice that it creates a chunk that in turn imports a function from the application entry point.

@evanw
Copy link
Owner

evanw commented Apr 3, 2021

Made a reproducation here: https:/zandaqo/esbuild_chunk

That's amazing! Thank you so much. I can reproduce it too, so I should be able to fix this.

@zandaqo
Copy link

zandaqo commented Apr 3, 2021

That's amazing! Thank you so much. I can reproduce it too, so I should be able to fix this.

No, thank you for the great tool, support, and overall experience here, the least we can do is complain about the inevitable bugs.

@evanw
Copy link
Owner

evanw commented Apr 3, 2021

A fully-reduced test case for the repo above is as follows:

  • entry.js: import('./a'); import('./b')
  • a.js: import { bar } from './shared'; bar()
  • b.js: import './shared'
  • shared.js: import { foo } from './foo'; export let bar = foo
  • foo/index.js: export function foo() {}
  • foo/package.json: { "sideEffects": false }

When you build this with --bundle --splitting --format=esm, the output files a-...js and chunk-...js both import symbols from each other, which is unexpected.

This bug is caused by "sideEffects": false somehow (not sure how yet) which means that you can work around the bug for now by using --tree-shaking=ignore-annotations until this bug is fixed.

@evanw
Copy link
Owner

evanw commented Apr 3, 2021

Can you verify that version 0.11.5 fixes this issue?

@zandaqo
Copy link

zandaqo commented Apr 4, 2021

@evanw All good here, 0.11.5 fixes it.

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

Successfully merging a pull request may close this issue.

4 participants