esbuild icon indicating copy to clipboard operation
esbuild copied to clipboard

Incorrect output order in bundle due to order of named exports

Open dzearing opened this issue 4 years ago • 9 comments

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 avatar Jul 09 '21 00:07 dzearing

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.

dzearing avatar Jul 09 '21 21:07 dzearing

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.

evanw avatar Jul 10 '21 15:07 evanw

@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.

dzearing avatar Jul 12 '21 17:07 dzearing

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

matthewjh avatar Sep 12 '22 17:09 matthewjh

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

cezarneaga avatar Oct 12 '22 09:10 cezarneaga

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

ChristopherHaws avatar Sep 23 '23 00:09 ChristopherHaws

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?

spillz avatar Mar 10 '24 15:03 spillz

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

seggewiss avatar Apr 26 '24 11:04 seggewiss