Using lazy imports seems to cause a size increase in the eagerly imported entry point.

[filipesilva]

• Rollup Version: 1.32.0

• Operating System (or Browser): Windows 10

• Node Version (if applicable): v12.4.0

• Link to reproduction (IMPORTANT, read below):

  • git clone https://github.com/filipesilva/rollup-aio-lazy
  • cd rollup-aio-lazy
  • yarn
  • yarn rollup --dir dist-full
  • Open src/app/custom-elements/element-registry.js and comment out this part:

      // Comment these out to remove lazy routes.
      // Start here:
      {
          selector: 'aio-announcement-bar',
          loadChildren: () => import('./announcement-bar/announcement-bar.module').then(m => m.AnnouncementBarModule)
      },
      {
          selector: 'aio-api-list',
          loadChildren: () => import('./api/api-list.module').then(m => m.ApiListModule)
      },
      {
          selector: 'aio-contributor-list',
          loadChildren: () => import('./contributor/contributor-list.module').then(m => m.ContributorListModule)
      },
      {
          selector: 'aio-file-not-found-search',
          loadChildren: () => import('./search/file-not-found-search.module').then(m => m.FileNotFoundSearchModule)
      },
      {
          selector: 'aio-resource-list',
          loadChildren: () => import('./resource/resource-list.module').then(m => m.ResourceListModule)
      },
      {
          selector: 'aio-toc',
          loadChildren: () => import('./toc/toc.module').then(m => m.TocModule)
      },
      {
          selector: 'code-example',
          loadChildren: () => import('./code/code-example.module').then(m => m.CodeExampleModule)
      },
      {
          selector: 'code-tabs',
          loadChildren: () => import('./code/code-tabs.module').then(m => m.CodeTabsModule)
      },
      {
          selector: 'live-example',
          loadChildren: () => import('./live-example/live-example.module').then(m => m.LiveExampleModule)
      }
      // Stop here.

  • yarn rollup --dir dist-nolazy
  • Compare the sizes of the main file in dist-full and dist-nolazy, see the full version is ~50kb larger:

  $ ls -la dist-full/main-rollup-91f3bcc1.js
  -rw-r--r-- 1 kamik 197609 1899825 Mar  2 19:40 dist-full/main-rollup-91f3bcc1.js
  
  $ ls -la dist-nolazy/main-rollup.js
  -rw-r--r-- 1 kamik 197609 1850516 Mar  2 19:42 dist-nolazy/main-rollup.js
  

  • If you are using VSCode, you can open those two files and run File: Compare Active File With... to see a diff between the two. Some of the extra retained symbols, like FocusKeyManager and A11yModule, are declared in main, unused there, exported, and only used in one of the lazy modules.

Expected Behavior

I expected that adding lazy imports would not add extra code to the eager code.

Actual Behavior

Adding lazy imports brought in extra code that should be in the lazy chunks.

[jakearchibald]

https://github.com/filipesilva/rollup-aio-lazy is empty.

Can you recreate the issue on https://rollupjs.org/repl/?

[filipesilva]

Oh gosh, I'm so sorry. I got everything together but then forgot to push! It should be there now.

[jakearchibald]

Can this be reduced at all? Creating a minimal reproduction will lead to this being fixed much quicker. You might even discover the source of the problem along the way! More info: https://git.io/fNzHA

[filipesilva]

I can pare down the input code but only by trial and error. I'm not familiar with the mechanisms that would lead to code being moved around and how it interacts with tree shaking so it is hard to make informed changes.

[jakearchibald]

Trial and error is absolutely good enough!

[filipesilva]

@jakearchibald here's a pared down repro, in the same repository:

---

rollup-aio-lazy

Repro for #3417.

Using lazy imports seems to cause a size increase in the eagerly imported entry point.

Repro steps:

• git clone https://github.com/filipesilva/rollup-aio-lazy
• cd rollup-aio-lazy
• yarn
• yarn test
• the script will create two builds and list their sizes:

-rw-r--r-- 1 kamik 197609 131900 Mar  3 11:27 dist/lazy-0a257795.js
-rw-r--r-- 1 kamik 197609     62 Mar  3 11:27 dist/main.js
-rw-r--r-- 1 kamik 197609 746421 Mar  3 11:27 dist/main-e77baaa0.js
-rw-r--r-- 1 kamik 197609  39060 Mar  3 11:27 dist-nolazy/main-nolazy.js

• the only difference between these two builds is that one has a dynamic import, and the other doesn't:

// src/main.js
import { MatButtonModule } from '@angular/material/button';
console.log(MatButtonModule)
import('./lazy')

// src/lazy.js
import * as i2 from "@angular/material/tabs";
console.log(i2)

// src/main-nolazy.js
import { MatButtonModule } from '@angular/material/button';
console.log(MatButtonModule)

• even though the module is dynamically imported, performing the dynamic import increases the size of the eagerly loaded module by 70k (~4k -> ~74k).
• having a dynamic import shouldn't alter the size of the eagerly loaded module

[filipesilva]

The source code is much smaller, but it's still importing large dependencies. The effect is much more dramatic than before though, going from 4k to 74k main bundle by adding a lazy module doesn't sound correct at all.

[jakearchibald]

I think this is a proper reduction.

Right now, Rollup cannot split individual modules. I'm not sure any build tool can. Here's an issue for it #2512

[lukastaegert]

Exactly, thanks for the spot-on analysis. Splitting large files is something we think about a lot but unfortunately all ideas we have so far would involve larger refactorings and are pushed back at the moment. So to rephrase the problem, you are statically importing a small export from a module and dynamically importing the remaining parts of the very same module. As Rollup cannot split a module, everything is included in the static import already.

[lukastaegert]

Note that without bundling, the same would still happen i.e. the static import would pull in the large file.

[filipesilva]

Thank you for the explanation. I understand now.

Angular ship 2 formats for ES2015 under different mainFields entry points:

• esm2015, containing multiple ES modules
• es2015, containing a single large ES module, bundled using rollup. This is the default we use for builds.

Following your explanation, I expected that using the esm2015 entry point would get around the problem, but that doesn't seem to happen. In fact, it gets larger.

• using es2015:

-rw-r--r-- 1 kamik 197609 131900 Mar  3 14:21 dist/lazy-a3df0392.js
-rw-r--r-- 1 kamik 197609     62 Mar  3 14:21 dist/main.js
-rw-r--r-- 1 kamik 197609 746421 Mar  3 14:21 dist/main-8ba4a765.js
-rw-r--r-- 1 kamik 197609  39060 Mar  3 14:21 dist-nolazy/main-nolazy.js

• using esm2015:

-rw-r--r-- 1 kamik 197609 136855 Mar  3 14:21 dist/lazy-8c9e40f4.js
-rw-r--r-- 1 kamik 197609    429 Mar  3 14:21 dist/main.js
-rw-r--r-- 1 kamik 197609 768190 Mar  3 14:21 dist/main-9531c628.js
-rw-r--r-- 1 kamik 197609  36787 Mar  3 14:22 dist-nolazy/main-nolazy.js

Am I missing something here? I updated the repro to use this main field.

[filipesilva]

Following @jakearchibald's previous reduction, I this one shows the same output even though the core.js module is split into core.js and core-massive.js.

// core.js
export * from './core-massive';

export function smallUtil() {
	console.log('smallUtil');
}

// core-massive.js
export function massiveUtil() {
	console.log('massiveUtil');
}

export function anotherMassiveUtil() {
	console.log('anotherMassiveUtil');
}

Since core-massive.js now contains the exports used by the lazy bundle, I expected them to only be imported there. But they are still eagerly loaded.

The only way to keep them separate seems to be directly importing from core-massive.js and not re-exporting the contents of core-massive.js from core.js at all, like in this repro.

[lukastaegert]

Try manually setting moduleSideEffects to false. There could be a static import between the split up modules. Also try with rollup@2.0.0-1 if it makes a difference

[filipesilva]

Using 2.0.0-1 moves the code used only in lazy chunks to the lazy chunk if it is in a separate module:

• repl with 2.0.0-1
• repl with 1.3.2

treeshake: { moduleSideEffects : false }, doesn't seem to make a difference with either 1.2.3 or 2.0.0-1.

I also tried with my bigger repro and saw a big improvement with 2.0.0-1:

• 1.3.2

-rw-r--r-- 1 kamik 197609 136855 Mar  3 14:21 dist/lazy-8c9e40f4.js
-rw-r--r-- 1 kamik 197609    429 Mar  3 14:21 dist/main.js
-rw-r--r-- 1 kamik 197609 768190 Mar  3 14:21 dist/main-9531c628.js
-rw-r--r-- 1 kamik 197609  36787 Mar  3 14:22 dist-nolazy/main-nolazy.js

• 2.0.0-1

-rw-r--r-- 1 kamik 197609 838265 Mar  3 15:20 dist/lazy-30f5abc2.js
-rw-r--r-- 1 kamik 197609     62 Mar  3 15:20 dist/main.js
-rw-r--r-- 1 kamik 197609  63844 Mar  3 15:20 dist/main-0fe59b91.js
-rw-r--r-- 1 kamik 197609  36419 Mar  3 15:20 dist-nolazy/main-nolazy.js

Using treeshake: { moduleSideEffects : false }, again made no difference. Probably because the Angular packages already have "sideEffects": false, in their package.json.

[lukastaegert]

Nice, I was hoping Rollup 2 would help here. It adds side-effect detection to improve the chunk split and this is a huge testament to its usefulness. It basically ready for release, it is just waiting for a last review of the core plugins with regard to breaking changes

[kzc]

Using 2.0.0-1 moves the code used only in lazy chunks to the lazy chunk if it is in a separate module:

Perhaps I'm not following this thread correctly... there is marked improvement with Rollup 2 regarding the placement of lazy chunks, but this particular limitation is by design?

[filipesilva]

Yes, I understand it is. At the time I was comparing builds flat ESM vs non-flat ESM builds so I just wanted to be specific.

[kzc]

Side note: there's some reliability issue with the Rollup REPL - an async race condition perhaps?

If I close Firefox 73 and reopen it with this issue's URL and click on the first REPL link in #3417 (comment), the Rollup REPL will not produce any output and the web console has an error:

"Error: Function called outside component initialization"

A subsequent reload of the same github URL and clicking on the REPL link will succeed in producing REPL output.

The REPL link in #3417 (comment) has the same problem with a newly restarted Firefox 73 instance.

Edit: To reproduce the REPL issue, the Firefox cache must be cleared completely upon restart or a private window must be launched.

[kzc]

I'm still not clear on the intended behavior of dynamic imports in Rollup.

Does Rollup 2 assume that all dynamic imports are opaque and could have side effects?

Is Rollup 2 expected to retain bar and baz in this example?

[lukastaegert]

Yes, because there is still no detection in Rollup 2 which properties are used or unused in a dynamic import. What Rollup 2 does is ignore empty imports if it cannot detect a side-effect in the imported module and it uses this to create more efficient chunks. More details here: #3369

[kzc]

Thanks for the clarification that there's no technical obstacle, and it's just a matter of implementation.

In my interpretation of the overview in #3369 it suggests that Rollup already deduces and records whether a given module is side effect free without the use of a package.json sideEffects hint or the use of moduleSideEffects. So dropping (or not including) bar and baz in my example ought to be possible. If I have time I'll take a look at this.

[lukastaegert]

True, the problem is that they are part of the public interface and it is not tracked yet in how far exports are used, which is a non-trivial thing to get right.

[lukastaegert]

E.g.

import('foo'); // no exports need to be included
import('foo').then(() => console.log('loaded')); // no exports need to be included
import('foo').then(({bar}) => console.log(bar)); // at least bar must be included
import('foo').then(ns => console.log(ns.bar)); // at least bar must be included
const importPromise = import('foo');
importPromise.then(({bar}) => console.log(bar)); // at least bar must be included

const logBar = ({bar}) => console.log(bar);
import('foo').then(logBar); // at least bar must be included

etc. This would be easier if we had object shape tracking first, which is a big thing I have been pushing away in favour of other things for some time. Also it would help to implement a deeper understanding of Promises into Rollup as well.

[lukastaegert]

Side note: there's some reliability issue with the Rollup REPL - an async race condition perhaps?

I had a look but the error seems to be deep inside Svelte. Maybe a Svelte bug, but hard to track in any case what is causing it, at least for me.

[jakearchibald]

This would be easier if we had object shape tracking first,

That sounds hard! I wonder if we might get there sooner via https://github.com/tc39/proposal-module-attributes#import-statements:

import { foo, bar } from './modules.js' as 'js-promise';

…where foo and bar are promises for the module's foo & bar exports.

[lukastaegert]

Maybe. I'm not sure I like module attributes but let's keep looking at this, I see a big danger that this will increase the syntactic gap between browsers and bundlers rather than making it smaller. I think at some point we will get to handling the cases I outlined correctly, at least in theory I have ideas where to start for those, but it is rather long-term. Still, existing code-bases would profit from this very much.

[kzc]

The module attributes proposal does not appear to aid dynamic import() much - it only allows for a coarse-grained second argument of type String to specify the return type. It is already known that a dynamic import implicitly returns a Promise, so analysis still must be done.

Object shape tracking would be the gold standard, but even without it I think some of the dynamic import scenarios could be special-cased and handled with a little effort - particularly the unused return value scenario, and the immediate import('module').then(...) scenario. Once the dynamic import is assigned to a variable it becomes considerably more difficult - at that point all exports would have to be assumed to be in use.

[jakearchibald]

The module attributes proposal does not appear to aid dynamic import() much - it only allows for a coarse-grained second argument of type String to specify the return type.

My suggestion was that it could be used instead of dynamic import, since it's easier to statically analyse. I agree that object shape tracking with regular import() would be better, unless it becomes really difficult for developers to stay on the optimised path.

[kzc]

instead of dynamic import

I thought that lazy imports require the use of dynamic import(). Perhaps I misinterpreted the module attributes proposal - is it providing a new alternate static syntax for dynamic import()?

[kzc]

There's a dumb but effective heuristic that could be employed without too much effort to eliminate many of the exports of dynamically import()ed side-effect-free modules. If a set of MemberExpression.property.names and Property.key.names are collected for the modules calling a given dynamic import('module'), then we assume the worst case scenario and must retain exports in the imported dynamic module matching any of those property names. Exported names that are not in the property name set of the dynamic import need not be retained - unless used in the imported module itself. Granted, dynamic module exports coincidentally matching common property names used for unrelated purposes may be unnecessarily retained, but a number of them that are not used could be dropped.

Edit: It might be prudent to include String Literal.values and MethodDefinition.key.names in the calling property name set for each dynamic import() in the rare event that they are indirectly used to do a dynamic lookup of an export.

[lukastaegert]

What about more complex computed properties?

const x = 3;
import('foo').then(ns => console.log(ns[`value${x}`]));

[lukastaegert]

But I must say the idea is quite ingenious.

Also, we not only need properties from the calling modules but from all modules as namespaces could be used in other modules. And if a namespace itself is just handed to an external callback, we are out of luck.

[kzc]

You don't see many such computed properties for dynamic import lookups in practise in the wild. There's a reason why people embraced statically analyzable import/export to avoid that sort of thing. But this dynamic module pruning could be an opt-in if that's a concern.

[kzc]

Also, we not only need properties from the calling modules but from all modules as namespaces could be used in other modules

I suppose that extra level of safety could be offered as a conservative option, but we might be able to trap such cases automatically. Limiting analysis to the calling module has the advantage of reducing the accidental property collision space resulting in less dynamically imported code needing to be bundled and/or chunked.

if a namespace itself is just handed to an external callback, we are out of luck.

It might be possible to escape certain dynamic import reference usages to include all exported symbols in the worst case scenario. Warnings could also be produced by Rollup in cases where this is happening to inform the user why certain dynamically imported code is being retained. Not 100% foolproof, but it should do the trick for most projects - particularly if the heuristic is an opt-in.

[jakearchibald]

@kzc

instead of dynamic import

I thought that lazy imports require the use of dynamic import(). Perhaps I misinterpreted the module attributes proposal - is it providing a new alternate static syntax for dynamic import()?

The proposal is:

import { foo, bar } from './modules.js' as 'something';

Where 'something' is an instruction for how to interpret the module. Browsers would understand certain values (like 'json' and 'css'), but build tools could have their own.

I was suggesting that Rollup would transpile:

import { foo, bar } from './modules.js' as 'js-promise';

to

const foo = import('./modules.js').then(m => m.foo);
const bar = import('./modules.js').then(m => m.bar);

The benefit of using a static import is you don't need object shape tracking, as import statements can be statically analysed.

But yeah, if object shape tracking is rock solid, a workaround like this isn't needed.

[kzc]

@jakearchibald I see what you mean now. I originally thought you were proposing a drop-in replacement for dynamic import(). This proposal would require users to restructure their code to have each (former) dynamic import take place unconditionally at module top level rather than on-demand, which is not always desirable. Even though the browser would likely cache each dynamically imported module, there would still be a bit of overhead for the multiple generated transpiled dynamic imports, such as foo and bar in the example above.

Rather than extending an ES proposal, it would be relatively straightforward to have Rollup recognize an annotation for dynamic import to state precisely which exports it is interested in. Such a hint would be backwards compatible with browsers and require minimal code change:

if (some_condition) {
    const mod = /*@rollup:import{foo,bar}*/ import("module");
    // ... unchanged existing logic using foo and bar ...
}

This hypothetical rollup annotation could also support wildcard patterns.

[kzc]

It just occurred to me that an annotation is not necessary for dynamic import if Rollup could recognize destructuring declaration assignments and drop unused exports in the module chunk accordingly:

        const {foo, bar} = await import("module");

[lukastaegert]

True, just supporting this pattern might be something we could support with rather low impact for now.

[kzc]

To recap, I think these are all the simple deterministic dynamic import patterns for side-effect-free modules:

// unreferenced
import('module'); // no exports need to be included
var unreferenced = import('module'); // no exports need to be included

// then arrows with optional destructuring parameters
import("module").then(() => console.log("loaded")); // no exports need to be included
import("module").then(({foo: f, bar}) => console.log(bar)); // at least foo and bar included

// then functions with optional destructuring parameters
import("module").then(function(){ console.log("loaded"); }); // no exports need be included
import("module").then(function({bar: b}){ console.log(bar); }); // at least bar need be included

// destructuring
let {foo: f, bar} = await import("module"); // at least foo and bar included
({foo, bar: b} = await import("module")); // at least foo and bar included

If all dynamic imports match one of the forms above for a given module, then it's just an additive set of exports that are to be retained. Any other dynamic import usage would result in retaining all exports in the module.

On a related note, any idea why var unused = is retained below?

$ echo 'var unused = import("./empty.mjs"), z = 0;' | dist/bin/rollup --silent

//→ -.js:
var unused = import('./empty-518cb9ca.js');

//→ empty-518cb9ca.js:

[lukastaegert]

On a related note, any idea why var unused = is retained below?

Because the right side has a side-effect and there is no logic yet to replace declarations with their side-effects in such a case.

Just a note: This discussion has nothing to do with the original issue other than it has something to do with dynamic imports. I would suggest to move it to a new issue.

[filipesilva]

I'm happy enough to have the issue hijacked really. I wasn't aware that there wasn't used/unused property detection in dynamic imports and that's definitely part of the ideal treeshaking story.

[kzc]

This will be my last post on the issue. After rescanning the thread I'm not sure what the resolution was - it seemed to be more of a discovery process. I honestly thought my points about side-effect-free dynamic imports were on-topic and advancing the goal of this project.

Because the right side has a side-effect

empty.mjs in the example above is a side-effect-free dynamic import - it's an empty file. The same output occurs with rollup@2.0.0-1. I must have misunderstood #3417 (comment):

What Rollup 2 does is ignore empty imports if it cannot detect a side-effect in the imported module and it uses this to create more efficient chunks.

[lukastaegert]

advancing the goal of this project

they definitely are. However the issue here was not which named exports are included in the dynamically imported module. The issue is a different one:

The main chunk was importing something from a file that had itself a dependency on the dynamically imported file (or one of its direct dependencies) even though nothing was USED from that dependency by the main chunk.

Still following strict execution dynamics, the code that was included by the dynamic import was now also included in the main chunk because previously, Rollup did not know too much about WHY code was included.

In the new version, however, the unused import will be removed from the main chunk if Rollup can be sufficiently certain that there are no side-effects. And hence it is attributed to the lazy chunk.

Detecting the actually used imports is also useful in its own right, but would have done nothing to solve the original issue.

[lukastaegert]

The original PR explains this in a little more detail with examples: #3369

[rollup-bot]

This issue has been resolved via #4952 as part of rollup@3.21.0. You can test it via npm install rollup.

[lukastaegert]

Ah, I think this is not actually resolved. At the core, this was about splitting modules that are shared between dynamic importers and import targets. Will reopen.

[lukastaegert]

Actually, looking at the conversation, it is solved, I just do not think that that was exactly the original issue...