evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
37.92k stars 1.13k forks source link

Incorrect output order in bundle due to order of named exports #1433

Open dzearing opened 3 years ago

dzearing commented 3 years ago

This issue repros with esbuild 0.12.15:

We're trying to bundle Outlook script with esbuild and we're seeing that files from the Pivot from the Fluent UI component library are being bundled in the wrong order. The Pivot imports PivotBase and exports a styled version of it. The expected order in the bundle output is that PivotBase gets defined, and then it is used to create/export the Pivot component. We are seeing the reverse: Pivot is defined and exported before PivotBase is defined (lower in the output file.) This causes a null reference in the browser.

We found this is due to the SliderBase.ts file importing SliderItem from ./index, which exports both Slider and SliderBase. Changing the import to ./SliderItem fixed the cycle and resolved the issue.

Expected: When cyclic dependencies occur (index -> SliderBase -> index), direct import order (Slider -> SliderBase) should control module order in the bundle (first define SliderBase, then define Slider.)

Resulted: The cyclic dependency caused the definition to be reversed (Slider, then SliderBase.)

Isolated repro here:

git clone https://github.com/dzearing/vite-fluent-bundling-issue
cd vite-fluent-bundling-issue
yarn
yarn dev

You should see an error in the console:

Uncaught TypeError: Cannot read property 'displayName' of undefined
    at styled (styled.tsx:123)
    at Pivot.tsx:12

If you modify src/Pivot.ts to remove the PivotBase export, or move it above the Pivot export, the bundle will be correctly ordered.

Note: the PivotItem import in PivotBase was fixed in Fluent UI 8.23.0.

dzearing commented 3 years ago

I have further pinpointed the issue:

Pivot.base.js imports PivotItem from ./index.js, which exports Pivot.base.js. There's a circular dependency here which I think is confusing the esbuild ordering.

evanw commented 3 years ago

Thanks for the detailed report. I don’t have time to look into this at the moment, sorry (I have an upcoming cross-country move in a few weeks). I hope to be able to look into this after my move is done.

dzearing commented 3 years ago

@evanw Thanks, no worries. I've pushed a fix to Fluent UI to remove the cycle from inside of Pivot.base.js, so that's a workaround on our end. We'll keep going.

matthewjh commented 1 year ago

I think this is still an issue. @evanw any chance you could take a look?

cezarneaga commented 1 year ago

hi @evanw could you please have a look into solving/providing a workaround strategy for this? thanks, C

ChristopherHaws commented 11 months ago

From what I am seeing in our app, modules are being resolved breadth-first instead of depth-first. For example:

// index.js
export * from './a/index.js';
export * from './b/index.js';

// a/index.js
export * from './one.js';
export * from './two.js';

// b/index.js
export * from './one.js';
export * from './two.js';

With this example, I am seeing it load in the following order:

/index.js
/a/index.js
/b/index.js
/a/one.js
/a/two.js
/b/one.js
/b/two.js

while the correct load order should be:

/index.js
/a/index.js
/a/one.js
/a/two.js
/b/index.js
/b/one.js
/b/two.js
spillz commented 6 months ago

From what I am seeing in our app, modules are being resolved breadth-first instead of depth-first.

I am seeing this too. How is this not an issue for more people? Flatter lib structure?

seggewiss commented 4 months ago

From what I am seeing in our app, modules are being resolved breadth-first instead of depth-first. For example:

// index.js
export * from './a/index.js';
export * from './b/index.js';

// a/index.js
export * from './one.js';
export * from './two.js';

// b/index.js
export * from './one.js';
export * from './two.js';

With this example, I am seeing it load in the following order:

/index.js
/a/index.js
/b/index.js
/a/one.js
/a/two.js
/b/one.js
/b/two.js

while the correct load order should be:

/index.js
/a/index.js
/a/one.js
/a/two.js
/b/index.js
/b/one.js
/b/two.js

Issue is still relevant and reproduceable