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

When using AMD output, use AMD for code-split chunks as well #5489

Open
onigoetz opened this issue Aug 10, 2017 · 24 comments
Open

When using AMD output, use AMD for code-split chunks as well #5489

onigoetz opened this issue Aug 10, 2017 · 24 comments

Comments

@onigoetz
Copy link

Do you want to request a feature or report a bug?

A new feature that would improve code-splitting with externals.

What is the current behavior?

Context :
An application which is build with multiple sub-applications which are each built with Webpack, and the "orchestration" of loading all this is done through requirejs.
One of the things with this platform, is that react is provided through requirejs so that each plugin can reuse a single global library instead of re-loading it for each app. (Any other library could be provided globally, I use react as the example here)
/context

When we generate a library in amd with some externals and code-splitting.
The thing is that externals are defined only at the main level even though it's the code-split part that need those externals.

Example :

// main.min.js
define(['react'], function(a) { /* Doesn't actually need react yet */ });

// 1.main.min.js
webpackJsonp([0],{3: function() { /* actually needs react */});

Wouldn't it be better if it was able to generate it this way ?

// main.min.js
define(function() { /* Doesn't actually need react yet */ });

// 1.main.min.js
define(["react"], function(a) {
  return {
    chunkIds: [0],
    moreModules: {3: function() { /* actually needs react */}}
  }
});

Advantages are :

  • no custom logic to load modules, requirejs knows how to do it.
  • external dependencies are loaded when they're actually needed
  • no need to specify a name for the jsonp function as it's done through requirejs

Problem with the current implementation:

  • externals might be loaded too early, which defeats the purpose of code splitting and lazy loading of those parts.

If the current behavior is a bug, please provide the steps to reproduce.

It isn't a bug, just the normal behaviour that could be more optimal

What is the expected behavior?

To load externals once they are needed

If this is a feature request, what is motivation or use case for changing the behavior?

My motivation is that I have an application that supports plugins, each of those can be implemented with any techology, React, jquery, vanilla js ... while they mostly use React today, we don't know what the future holds for us. Which is why we want to be able to lazily load libraries once they are actually used.

Please mention other relevant information such as the browser version, Node.js version, webpack version and Operating System.

We use webpack 3.5.2

@mspoerer
Copy link

+1

@divdavem
Copy link

This feature request would be useful, for example, to develop Grafana plugins.
Grafana uses SystemJS (which is compatible with AMD) to load plugins.
So, if I want to use Webpack to package a set of Grafana plugins (such as a Grafana App plugin), I need to have multiple entry points (one for each plugin), all of them in the AMD format (so that SystemJS can load them).
But if they have common dependencies, I don't want them to be duplicated in each entry point bundle... but if I use a common chunk, SystemJS currently does not know about it and will not wait for this chunk to be loaded before executing the loaded plugin. But if Webpack created this chunk in the AMD format and used AMD to load it, SystemJS would correctly wait for it to be loaded before executing the plugin that depends on it.

@ryx
Copy link

ryx commented Jun 1, 2018

We are in the middle of migrating a requirejs-based application with custom bundling to a React SPA with webpack4. With AMD output we could load the split chunks through our old loader, too. That would make it incredibly more convenient to create a commons package holding our third party dependencies, while keeping our old module system in place until we are finished. So this would make webpack much more appealing to people coming from another loader.

Right now the only solution for us is to use one fat bundle for React/3rdParty and lose all advantages of dedicated bundling, unless we switch to webpack as loader (which is no option atm).

@DeTeam
Copy link

DeTeam commented Oct 16, 2018

@onigoetz have you managed to get far with this issue: maybe find another approach/plugin/workaround/tool? :)

@onigoetz
Copy link
Author

Hello @DeTeam No I didn't get far on this issue yet, currently I still have this sub-optimal situation, I definitely want to look for a better approach but don't have any scheduled time to do so. I'll have some time off around Christmas, I'll try to have a look at that moment.

@marcelmokos
Copy link

I have a workaround. I am using StatsWriterPlugin to get dependencies and then I convert generated files using string manipulation. It is ugly but it is working for a Liferay use case where Liferay is doing dependency injection based on its config.

  setOutput({
      library: "[name]",
      libraryTarget: "amd",
      umdNamedDefine: true,
    }),

// ...

new StatsWriterPlugin({
        filename: "config.json",
        fields: ["assetsByChunkName", "entrypoints"],
        transform({assetsByChunkName, entrypoints}, opts) {
          const getName = path => path.replace(/\.js$/, "");

          const meta = Object.entries(assetsByChunkName).reduce(
            (acc, [name, path]) => {
              if (name.startsWith("commons~")) {
                return {
                  ...acc,
                  [getName(path)]: {
                    name: getName(path),
                    path,
                    dependencies: [],
                  },
                };
              }

              const dependencies = entrypoints[name].assets
                .filter(dependency => dependency !== path)
                .map(getName);

              return {
                ...acc,
                [name]: {name, path, dependencies},
              };
            },
            {},
          );

          return JSON.stringify(meta, null, 2);
        },
      }),
function convertToFakeAMD() {
  console.log(`🐣=>️🐥 converting modules to fake AMD modules`);

  Object.entries(configJson)
    .filter(([_, {dependencies}]) => dependencies.length === 0)
    .forEach(([_, entry]) => {
      const filePath = path.join(workPath, entry.path);
      const fileContent = fs.readFileSync(filePath, "utf8");

      const fileContentAMD = `define("${
        entry.name
      }",[],function(){${fileContent}
});`;

      fs.writeFileSync(filePath, fileContentAMD);

      console.log(
        `🐥 [${filePath.replace(workPath, "buildPath")}] updated to AMD module`,
      );
    });
}

@Aghassi
Copy link

Aghassi commented Nov 18, 2018

@TheLarkInn I know this is a "nice to have" feature as tagged, but if it is something I (or others) can help contribute, it would be a huge help for the scale we are working at. I'm looking through the code, and I believe the main area where issue occurs is

const externals = chunk.getModules().filter(m => m.external);
const externalsDepsArray = JSON.stringify(
externals.map(
m => (typeof m.request === "object" ? m.request.amd : m.request)
)
);
const externalsArguments = externals
.map(
m => `__WEBPACK_EXTERNAL_MODULE_${Template.toIdentifier(`${m.id}`)}__`
)
.join(", ");

Since the code does lack a bit of documentation, I am doing my best to infer. Based on what I see, it seems that whatever is listed as externals in the bundle is just automatically put in the main file template header, regardless of if it is needed or not for that chunk. Is that correct? If so, then my follow up question is are there any mechanisms in place that would allow Webpack to infer what externals apply to each code split chunk? I'm happy to try and help contribute back this fix, but I also am trying to understand the inner workings before hand so I'm not just shooting in the dark.

@DeTeam
Copy link

DeTeam commented Nov 19, 2018

@Aghassi I spent a bit of time on reading webpack internals and feel like there're few things to check out:

  • AMD template plugin (the one you mentioned)
  • lib/web/JsonpChunkTemplatePlugin.js — this one's gonna be used for separate chunks, as I understand

One way to tackle this would be to:

  1. Create custom plugins for chunk and and output templates
  2. Experiment with how the templates work until you get the desired output (AMD with splitchunks)
  3. Change existing internal templates to what's implemented

^^^ I wanted to take this route, but still learning some of the internals of webpack for a better background knowledge

@mfreeman-xtivia
Copy link

@marcelmokos do you have a full working example of the webpack config that you used to build the config.js file you describe above? if so could you share as we have a common interest in solving this problem as well

@marcelmokos
Copy link

First I generate App entry points.
generateAppEntryPoints.js

// @flow
/* eslint-disable no-console */

const fs = require("fs");
const glob = require("glob");
const pify = require("pify");
const path = require("path");

const getFilePathsForPattern = async pattern => pify(glob)(pattern);
const getAppName = path =>
  path
    .replace("\\", "/")
    .replace("//", "/")
    .split("/")[1];

const getEntry = async (pattern = "applications/**/src/index.js") => {
  const filePaths = await getFilePathsForPattern(pattern);

  return filePaths.reduce(
    (acc, filePath) => ({
      ...acc,
      [getAppName(filePath)]: [path.resolve(process.cwd(), filePath)],
    }),
    {},
  );
};

async function generateAppEntryPoints(
  filePath = path.join(
    process.cwd(),
    "applications",
    "webpack-entry-points.json",
  ),
) {
  const entry = await getEntry();
  console.log(`🥚 creating app entry points json [${filePath}]`);

  fs.writeFileSync(filePath, JSON.stringify(entry, null, 2));
}

module.exports = generateAppEntryPoints;

Then I run webpack that will run StatsWriterPlugin.
webpack.config.js

/* eslint-disable global-require */
require("../../../../../ableneo/tools/scripts/envSetup");
require("dotenv").config();

const {resolve} = require("path");
const webpack = require("webpack");
const {StatsWriterPlugin} = require("webpack-stats-plugin");

const postcssPresetEnv = require("postcss-preset-env");

const {
  createConfig,
  match,

  // Feature blocks
  babel,
  css,
  sass,
  devServer,
  url,
  postcss,
  uglify,

  // Shorthand setters
  addPlugins,
  setEnv,
  entryPoint,
  env,
  setOutput,
  sourceMaps,
  customConfig,
  performance,
} = require("webpack-blocks");

const preset = {
  cssModules: {
    localIdentName: "[name]__[local]___[hash:base64:5]",
  },
  postcss: {
    ident: "postcss",
    plugins: () => [
      require("postcss-flexbugs-fixes"),
      require("postcss-import")(),
      postcssPresetEnv({
        browsers: [
          ">1%",
          "last 4 versions",
          "Firefox ESR",
          "not ie < 9", // React doesn't support IE8 anyway
        ],
      }),
    ],
  },
};

const getEntry = () => {
  try {
    // eslint-disable-next-line import/no-unresolved
    return require("../../applications/webpack-entry-points.json") || {};
  } catch (error) {
    return {};
  }
};

const config = createConfig([
  entryPoint(getEntry()),
  setOutput({
    filename: "[name].[chunkhash].js",
    chunkFilename: "[id].[chunkhash].js",
    path: resolve(__dirname, "dist"),
  }),
  babel(),
  match(
    ["*.css", "!*node_modules*"],
    [css.modules(preset.cssModules), postcss(preset.postcss)],
  ),
  match(
    ["*.scss", "!*node_modules*"],
    [css.modules(preset.cssModules), postcss(preset.postcss), sass()],
  ),
  // will load images up to 10KB as data URL
  match(
    ["*.gif", "*.jpg", "*.jpeg", "*.png", "*.svg", "*.webp"],
    [url({limit: 30000})],
  ),
  setEnv({
    NODE_ENV: process.env.NODE_ENV,
  }),
  addPlugins([
    new webpack.DefinePlugin({
      __DEV__: JSON.stringify(process.env.NODE_ENV !== "production"),
      STYLEGUIDIST_LIFERAY_THEME_SERVER_PORT: JSON.stringify(
        process.env.STYLEGUIDIST_LIFERAY_THEME_SERVER_PORT,
      ),
    }),
  ]),

  performance({
    hints: false,
  }),

  env("development", [
    devServer({
      // Show full-screen overlay in the browser on compiler errors or warnings
      overlay: true,
    }),

    sourceMaps("cheap-module-eval-source-map"),
  ]),
  env("production", [
    uglify({
      parallel: true,
      cache: true,
      uglifyOptions: {
        compress: {
          warnings: false,
        },
        output: {
          comments: false,
        },
      },
    }),

    setOutput({
      library: "[name]",
      libraryTarget: "amd",
      umdNamedDefine: true,
    }),

    addPlugins([
      new webpack.LoaderOptionsPlugin({minimize: true}),

      new StatsWriterPlugin({
        filename: "config.json",
        fields: ["assetsByChunkName", "entrypoints"],
        transform({assetsByChunkName, entrypoints}, opts) {
          const getName = path => path.replace(/\.js$/, "");

          const meta = Object.entries(assetsByChunkName).reduce(
            (acc, [name, path]) => {
              if (name.startsWith("commons~")) {
                return {
                  ...acc,
                  [getName(path)]: {
                    name: getName(path),
                    path,
                    dependencies: [],
                  },
                };
              }

              const dependencies = entrypoints[name].assets
                .filter(dependency => dependency !== path)
                .map(getName);

              return {
                ...acc,
                [name]: {name, path, dependencies},
              };
            },
            {},
          );

          return JSON.stringify(meta, null, 2);
        },
      }),
    ]),

    customConfig({
      stats: {
        assetsSort: "!size",

        // `webpack --colors` equivalent
        colors: true,

        // Sort the chunks by a field
        // You can reverse the sort with `!field`. Default is `id`.
        chunksSort: "!size",

        // Add chunk information (setting this to `false` allows for a less verbose output)
        chunks: true,

        // Add the origins of chunks and chunk merging info
        chunkOrigins: false,

        // Add built modules information
        modules: true,

        // Sort the modules by a field
        // You can reverse the sort with `!field`. Default is `id`.
        modulesSort: "!size",

        // Set the maximum number of modules to be shown
        maxModules: 10,
      },
    }),

    customConfig({
      optimization: {
        splitChunks: {
          cacheGroups: {
            commons: {
              chunks: "initial",
              minChunks: 2,

              minSize: 10000,
              maxInitialRequests: 64, // number of chunks per request (more lead to overall smaller sizes)
            },
          },
        },
      },
    }),
  ]),
]);

module.exports = config;

Then I create liferay config.
createLiferayConfig.js

/* eslint-disable no-console,no-undef,import/no-dynamic-require */

const fs = require("fs");
const glob = require("glob");
const pify = require("pify");
const path = require("path");

const generateLiferayConfig = require("./generateLiferayConfig");

const args = require("../utils/args");

const workPath = args[0];
const configJson = require(path.join(workPath, "config.json"));

function generateAppEntryPoints(filePath = path.join(workPath, "config.js")) {
  const config = generateLiferayConfig(configJson);

  console.log(
    `🐣 creating liferay config [${filePath.replace(workPath, "buildPath")}]`,
  );

  fs.writeFileSync(filePath, config);
}

generateAppEntryPoints();

function convertToFakeAMD() {
  console.log(`🐣=>️🐥 converting modules to fake AMD modules`);

  Object.entries(configJson)
    .filter(([_, {dependencies}]) => dependencies.length === 0)
    .forEach(([_, entry]) => {
      const filePath = path.join(workPath, entry.path);
      const fileContent = fs.readFileSync(filePath, "utf8");

      const fileContentAMD = `define("${
        entry.name
      }",[],function(){${fileContent}
});`;

      fs.writeFileSync(filePath, fileContentAMD);

      console.log(
        `🐥 [${filePath.replace(workPath, "buildPath")}] updated to AMD module`,
      );
    });
}

convertToFakeAMD();

That is calling following function to generateLiferayConfig.js

/*
export type Module = {
  name: string,
  dependencies: Array<string>,
  path: string,
};

export type Modules = {[string]: Module};
*/

const getModule = ({name, dependencies, path}) =>
  `{
  name: ${JSON.stringify(name)},
  dependencies: ${JSON.stringify(dependencies)},
  path: MODULE_PATH + "/${path}"
}`;

const generateLiferayConfig = json =>
  Object.entries(json).reduce(
    (acc, [key, module]) =>
      `${acc}Liferay.Loader.addModule(${getModule(module)});
`,
    "",
  );

module.exports = generateLiferayConfig;

I am not im now on different project. Here is the part of the package.json I do not have a public running example I can share for now.

"entry-points:generate": "cross-env NODE_ENV=production node ./common/applicationsBuild/createAppEntryPoints.js",
"prebuild": "yarn run entry-points:generate",
"build": "cross-env NODE_ENV=production webpack --mode=production --config ./common/webpack/webpack.config.js",
"build:liferay-js-config": "cross-env NODE_ENV=production node ./common/applicationsBuild/createLiferayConfig.js"

@Aghassi
Copy link

Aghassi commented Oct 25, 2019

@DeTeam So... I've been digging into this given the current structure of Webpack for v5 beta. I have created a plugin that can at least know which chunk an external should be loaded in (thanks to a lot of pointers and help from @sokra). However, the JSONP callback to invoke chunks is not helpful in the current state. I'm guessing, given the current structure of Webpack's bootstrapping mechanism, that the existing way of bootstrapping loads externals up front so that the webpack module bootstrapping code can avoid callback hell of any sort. Where I'm currently stuck is when webpack tries to load an external module, it calls the following:

/******/ 	
/******/ 	// The require function
/******/ 	function __webpack_require__(moduleId) {
/******/ 		// Check if module is in cache
/******/ 		if(__webpack_module_cache__[moduleId]) {
/******/ 			return __webpack_module_cache__[moduleId].exports;
/******/ 		}
/******/ 		// Create a new module (and put it into the cache)
/******/ 		var module = __webpack_module_cache__[moduleId] = {
/******/ 			i: moduleId,
/******/ 			l: false,
/******/ 			exports: {}
/******/ 		};
/******/ 	
/******/ 		// Execute the module function
/******/ 		__webpack_modules__[moduleId](module, module.exports, __webpack_require__);
/******/ 	
/******/ 		// Flag the module as loaded
/******/ 		module.l = true;
/******/ 	
/******/ 		// Return the exports of the module
/******/ 		return module.exports;
/******/ 	}

Specifically, the following is what is problematic

/******/ 		// Execute the module function
/******/ 		__webpack_modules__[moduleId](module, module.exports, __webpack_require__);

This call normally expects the chunk to have your external mapped to an eval with the module.exports ready. In the main chunk this looks like

/***/ "react":
/***/ ((module) => {

eval("module.exports = __WEBPACK_EXTERNAL_MODULE_react__;\n\n//# sourceURL=webpack:///external_%22react%22?");

/***/ })

However, __WEBPACK_EXTERNAL_MODULE_react__ is hydrated from the IIFE at the top of the main chunk. Without that in lazy loaded chunks, we can't fetch the module and assign it properly.

Most lazy loaded chunks are structured as

(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["a"], {
 /** Object mapping **/
}]);

In this scenario, a maps to an object map where each key in the map has an object. So, to the above with react, when react is asked to load, it runs the IIFE that does the eval that assumes the module is already loaded.

So to sum it up, it's more than just a plugin and this is where I'm stuck. Unless I'm missing something, the plugin would also need to modify the webpack boot loader code to handle callback loading of deferred modules somehow. I haven't yet been able to piece together how I could sequence something like this. For example, we use requirejs, which doesn't offer any Promise API unfortunately, so waiting on the module to be returned is a little more annoying.

@Aghassi
Copy link

Aghassi commented Nov 11, 2019

An update to the above, I think I have a working solution for a plugin that works with webpack 5. Shoutout to @krohrsb for helping me get over the jsonp issue. I essentially had to add a timeout that checks if a chunk has loaded. I need to cleanup the code some more and do some actual testing with production code, but I'll be sure to post here once I am sure I have something.

@ianschmitz
Copy link

@Aghassi you may want to collab on #8837 perhaps?

@Aghassi
Copy link

Aghassi commented Nov 15, 2019

@ianschmitz is that PR still active? It looked stale. I was hoping to finish putting together this plugin and then using it as a reference to start a PR. Either way, I’d like webpack core to have this logic eventually.

@Aghassi
Copy link

Aghassi commented Jan 30, 2020

@sokra @TheLarkInn I've made my work public here https://github.com/Aghassi/umd-treeshake-externals-webpack-plugin. This works to solve this issue, and it is compatible with Webpack 5 only at the moment. The code is very messy, and I haven't had time to clean it up. However, I believe it can be used by the core team to solve this issue if you would like to work together on bringing this change into Webpack. Shoutout to @krohrsb for helping me with the payload waiting logic.

@artola
Copy link

artola commented Jul 8, 2020

@Aghassi This code works as expected with the beta version 9, but not longer anymore on 21. @sokra is it possible to revive this issue and may be find a solution looking forward (or even v4) ?

@Aghassi
Copy link

Aghassi commented Sep 29, 2020

@artola I don't work on that project anymore as it was for my last job. That being said, I can see if I have time to update it for the current RC. I don't see why it wouldn't be possible. Probably just some internal methods need changing. Happened a few times during the beta period

@ianschmitz
Copy link

For clarity to the maintainers, this affects more than just libraryTarget: 'amd'. From my tests all the following experience the same issue of hoisting split chunk external dependencies to the main chunk:

  • amd
  • amd-require
  • umd
  • system

@privatenumber
Copy link
Contributor

This is somewhat attainable now by creating your own custom chunk entry-point and creating a dependency to it via dependOn.

@artola
Copy link

artola commented Jan 23, 2021

@privatenumber Do you mean manually creating the chunks? (not doable in a big app with a bunch of dependencies)

@privatenumber
Copy link
Contributor

Thats why I said somewhat 😅

If your chunk is a vendors chunk, it's very doable.
I think it's still doable to manually chunk split-points as entry-points but more complex.

@Soombra
Copy link

Soombra commented Mar 23, 2021

Is there any progress ?

@Aghassi
Copy link

Aghassi commented Mar 25, 2021

I no longer work on this problem given my current role. The only way forward is to update the plugin one beta at a time to webpack 5, then look to upstream the changes. I currently don't have the mental energy or the cycles to take this on in addition to my main work and life. I'd love to see it get over the line, and ideally have the PoC that my repo represents make it's way into Webpack officially. That being said, it's gonna take some time from invested parties to get the example over the line and upstream.

@sysofwan
Copy link

This is somewhat attainable now by creating your own custom chunk entry-point and creating a dependency to it via dependOn.

@privatenumber I tried doing this using dependOn but it doesn't seem to work due to issue #10537

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests