BUG report and Feature request: Support AMD bundling with preserveModules on file(bundle.js)

[izelnakri]

Feature Use Case

Currently rollup acts as an advanced bundler(static + npm import resolver across multiple files, treeshaking, sourcemaps) and module formatter/transpiler(cjs <-> amd <-> iife etc), it also serves a very nice plugin system. Recently it seems experimentalPreserveModules became stable, however it misses preserving modules on a single file bundles([dir | file ] -> bundle.js), in fact currently rollup throws an error if preserveModules gets used without a dir configuration or used with file configuration.

I think this is an important feature that opens up new possibilities for rollup(for example, it can allow rollup to be the default packager for the next generation ember-cli) and most work seems to be already done. When a user selects preserveModules with dir, rollup can correctly lookup the dependency graph and transpile files and dependencies to the target module format, while doing so however it also misses moduleNames for each AMD module:

preserveModules: today:

// in rollup.config.js

export default {
  input: 'main.js',
  output: {
    dir: 'dist',
    format: 'amd',
    amd: {
      id: 'frontend',
    },
  },
  preserveModules: true
}

// before: main.js
import Another from './another';
import Component from '@glimmer/component';

console.log('Component is');
console.log(Component);
console.log(Another);

// before: another.js
import Component from '@glimmer/component';
console.log('calling another');
console.log(Component);
export default {
  firstName: 'Foo',
  lastName: 'Bar'
}

===========
AFTER: rollup -c :

// in dist/main.js -> AMD module name is missing! Dependency naming is correct
// however the dependency path should also be customizable, ex: 'frontend/another.js'
define(['@glimmer/component', './another.js'], function (Component, __chunk_1) { 'use strict';
	Component = Component && Component.hasOwnProperty('default') ? Component['default'] : Component;

	console.log('Glimmer is');
	console.log(Component);
	console.log(__chunk_1.default);
});

// dist/another.js -> AMD module name is missing!
define(['exports', '@glimmer/component'], function (exports, Component) { 'use strict';
  Component = Component && Component.hasOwnProperty('default') ? Component['default'] : Component;

  console.log('calling another');
  console.log(Component);
  var Another = {
    name: 'Foo',
    lastName: 'Bar'
  };

  exports.default = Another;
});

Feature Proposal

For preserveModules with file: 'bundle.js'. Given:

// before: main.js
import Another from './another';
import Component from '@glimmer/component';

console.log('Component is');
console.log(Component);
console.log(Another);

// before: another.js
import Component from '@glimmer/component';
console.log('calling another');
console.log(Component);
export default {
  firstName: 'Foo',
  lastName: 'Bar'
}

Result should be:

// after: bundle.js

define('@glimmer/component' /* @glimmer/component transpilation here, just like how its done with dir: folder option, but also with npm module names as AMD module names */ );

define('frontend/another.js', ['exports', '@glimmer/component'], function (exports, Component) { 'use strict';
  Component = Component && Component.hasOwnProperty('default') ? Component['default'] : Component;

  console.log('calling another');
  console.log(Component);
  var Another = {
    name: 'Foo',
    lastName: 'Bar'
  };

  exports.default = Another;
});

define('frontend/main', ['@glimmer/component', 'frontend/another.js'], function (Component, __chunk_1) { 'use strict';
	Component = Component && Component.hasOwnProperty('default') ? Component['default'] : Component;

	console.log('Glimmer is');
	console.log(Component);
	console.log(__chunk_1.default);
});

require('frontend/main');

[lukastaegert]

Hi @izelnakri, thanks for this issue.

So with your proposal, file only really makes sense in an AMD (or SystemJS) situation?

I am not sure this is easily supportable as this would be a completely new rendering mode (i.e. concatenating everything) but something that is theoretically possible. Nevertheless my feeling is that this would make more sense to be solved by a plugin, at least the concatenation part, or do you see any issues here? The generateBundleHook should have all the necessary information (though I just noticed the website forgets to mention that it also contains a code property that contains the generated code). In that case, all that is left would be a way to assign amd.ids.

I assume that is the reason why you classified this as a bug, though my understanding is that when using one file per module, these ids are not needed and possibly even discouraged. But I see that there are several possible approaches here:

• allow output.amd.id to be a function to generate these ids from the chunk ids
• allow a value of output.amd.id: true to just add chunk ids as amd ids? (Though I am not sure about potential side-effects).

In any case something else I noticed is that we should probably remove the .js extensions from amd imports as having a .js extension does change the resolution logic (but this probably has little bearing on your issue).

[izelnakri]

thanks for the quick response @lukastaegert!

yes file only makes sense for AMD and SystemJS definitions.

I agree that concatenation would be more challenging, first ideal step would be to adjust the dir option behavior to include module names.

I would like to use file option without an extra plugin for my case since it requires no change to the default provided APIs. Also, plugins will require their own updates and maintainance, for example this plugin received no updates for 7 months and has security vulnerabilities: https://www.npmjs.com/package/rollup-plugin-node-globals. So I think for this situation having the behavior in rollup is a better choice.

I'll look into the API's you've mentioned perhaps I can build something on it whenever I find time, meanwhile please let me know if there are any updates/PR from you. Thanks again for the library and your consideration!

[lukastaegert]

Also, plugins will require their own updates and maintainance

The point is, Rollup itself also requires maintenance. If I spend time maintaining things that could be maintained by others then this will prevent other core features from being developed. By proposing to build a plugin I am also proposing that I will not be the one building the plugin, and at the moment I am more or less the core team. But I will be very happy to support you in any way I can because then it will be a net win for the whole ecosystem.

I hope I will find some time soon to look into the core issues I listed, mainly a way to add amd ids and probably have another look at file extensions.

[aapoalas]

Piggybacking on this discussion: The power to give output AMD modules IDs programmatically would definitely be quite useful.

I had a case where I would've wanted to give n input modules to Rollup, have Rollup then bundle these together into single AMD modules as much as possible while splitting into chunks any common imports between the input modules, and have all of these modules then follow some naming scheme eg. "my-specific-loader!/not/the/filesystem/path/to/module".

[shellscape]

I'm going to contact the author to see if they'd be cool with brining rollup-plugin-node-globals into the Rollup org so that it can be kept up to date.

[lukastaegert]

Please have a look at the discussion in #2881. There was at least work on an alternative plugin by @manucorporat but I do not know what the current state is

[lukastaegert]

As for the original issue, as AMD ids are broken anyway for code-splitting, may Plan would be to

• throw an error when a string is used for the output.amd.id option in a code-splitting build
• allow a new value output.amd.id: true that will auto-generate ids based on the chunk names and use these ids for relative imports. But of course no time frame, sorry. Then it would be possible to concatenation chunks as you like via plugin or otherwise.