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

Application with multiple iframes performances problems on hot reload #2969

Open
1 of 2 tasks
paztis opened this issue Jan 19, 2021 · 28 comments
Open
1 of 2 tasks

Application with multiple iframes performances problems on hot reload #2969

paztis opened this issue Jan 19, 2021 · 28 comments

Comments

@paztis
Copy link

paztis commented Jan 19, 2021

  • Operating System: MacOS BigSur
  • Node Version: 14.15
  • NPM Version: 6.14.8
  • webpack Version: 4.44.2
  • webpack-dev-server Version: 3.11.1
  • Browser: chrome 88.0.4324.87
  • This is a bug
  • This is a modification request

Code

// webpack.config.js
{
    output: {
        library: 'viewDesignerApp',
        path: 'dist',
        publicPath: '/my-app/',
        libraryTarget: 'umd'
    },
    entry: {
        'index': 'src/index',
        'index-frame1': 'src/index-frame1',
        'index-frame2': 'src/index-frame2'
    },
    plugins: [
        new HtmlWebpackPlugin({
            filename: 'index.html',
            chunks: ['index']
        }),
        new HtmlWebpackPlugin({
            filename: 'index-frame1.html',
            chunks: ['index-frame1']
        }),
        new HtmlWebpackPlugin({
            filename: 'index-frame2.html',
            chunks: ['index-frame2']
        }),
    ],
    optimization: {
        splitChunks: {
            chunks: 'all'
        }
    },
    devServer: {
        contentBase: ['dist'],
        contentBasePublicPath: '/my-app/',
        compress: true,
        publicPath: '/my-app/',
        port: 3001
    }
}

src/index.js

import {render} from 'react-dom';

const rootNode = document.getElementById('root');
render(
    <div>
        <iframe src="./index-frame1.html" />
        <iframe src="./index-frame2.html" />
    </div>,
    rootNode
);

src/index-frame1.js

import {render} from 'react-dom';

const rootNode = document.getElementById('root');
render(
    <div>SUB FRAME 1</div>,
    rootNode
);

src/index-frame2.js

import {render} from 'react-dom';

const rootNode = document.getElementById('root');
render(
    <div>SUB FRAME 2</div>,
    rootNode
);

Expected Behavior

I've an application that contains 2 sub iframes
Webpack generates 3 html files for the application
In dev mode, in case of code change, only the top level window might listne the events / try to refresh the page
or the main window need a way to advert the subframes to avoid listening

Actual Behavior

in dev mode, all the frames + root window are listening the hot reload changes.
In case of code change, the frames + root window tries to reload the page in same time
Chrome devTools frequently crashes because of to many logs (3x the progress logs) + interrupted sourcemaps decoding

For Bugs; How can we reproduce the behavior?

create an app with an iframe, both pointing to html pages managed by webpack

For Features; What is the motivation and/or use-case for the feature?

@alexander-akait
Copy link
Member

alexander-akait commented Jan 19, 2021

Do not ignore section how we can reproduce it, i.e. core and configuration, otherwise we can't help

In dev mode, in case of code change, only the top level window might listne the events / try to refresh the page
or the main window need a way to advert the subframes to avoid listening

Expected, why do you avoid changes here?

@paztis
Copy link
Author

paztis commented Jan 19, 2021

You want me to produce a full application to explain you the problem ?

Expected, why do you avoid changes here?

In case of liveReload (page reload), the subframe is destroyed during the top window unload
Then a new subframe is created.
Devtools encounter problems because it starts to decode sourcemap then destroy and load a new frame

I search a way to inform the subframes not to listen to socket events or at least to not refresh code.
But if I disable hot or liveReload, I do it also for top level window.

There's no way to provide different configs for each page produced by the same webpack build

@alexander-akait
Copy link
Member

You want me to produce a full application to explain you the problem ?

Minimum or configuration

@paztis
Copy link
Author

paztis commented Jan 19, 2021

original comment updated with webpack config and index files

@alexander-akait
Copy link
Member

You need to use injectHot as function and return false for index-frame1 and index-frame2

@paztis
Copy link
Author

paztis commented Jan 20, 2021 via email

@paztis
Copy link
Author

paztis commented Jan 26, 2021

any other idea @alexander-akait ?

@alexander-akait
Copy link
Member

We need improve injectClient and injectHot and pass entry to callback

@paztis
Copy link
Author

paztis commented Jan 26, 2021

Great that was my idea.

@alexander-akait
Copy link
Member

Feel free to send a PR

@paztis
Copy link
Author

paztis commented Jan 28, 2021

I've take a look at it
I correctly see how to do it for webpack 4 (in lib/utils/devServerPlugin.js) in prependEntry, as I control all the chunks

const prependEntry = (originalEntry, additionalEntries) => {
      if (typeof originalEntry === 'function') {
        return () =>
          Promise.resolve(originalEntry()).then((entry) =>
            prependEntry(entry, additionalEntries)
          );
      }

      if (typeof originalEntry === 'object' && !Array.isArray(originalEntry)) {
        /** @type {Object<string,string>} */
        const clone = {};

        Object.keys(originalEntry).forEach((key) => {
          // entry[key] should be a string here
          const entryDescription = originalEntry[key];
        // ===========> job to do to decide to inject or not, depending of 'key' chunk
          clone[key] = prependEntry(entryDescription, additionalEntries);
        });

        return clone;
      }

      // in this case, entry is a string or an array.
      // make sure that we do not add duplicates.
      /** @type {Entry} */
      const entriesClone = additionalEntries.slice(0);
      [].concat(originalEntry).forEach((newEntry) => {
        if (!entriesClone.includes(newEntry)) {
          entriesClone.push(newEntry);
        }
      });
      return entriesClone;
    };

But in webpack 5 mode, I don't see how to do it

compiler.hooks.make.tapPromise('DevServerPlugin', (compilation) =>
        Promise.all(
          additionalEntries.map(
            (entry) =>
              new Promise((resolve, reject) => {
                compilation.addEntry(
                  compiler.context,
                  EntryPlugin.createDependency(entry, {}),
                  {}, // global entry
                  (err) => {
                    if (err) return reject(err);
                    resolve();
                  }
                );
              })
          )
        )
      );

Any idea of it ?

@alexander-akait
Copy link
Member

We should improve our check, yes, it is not implemented

@paztis
Copy link
Author

paztis commented Feb 2, 2021

until this is developed, is there's any way to do it through custom plugin I can wrote or something else ? I'm using webpac-dev-server@3.11.2 and webpack@4.

I know how to do it inside your code for webpack 4, but useless to create a PR that didn't cover the webpack 5 case, or it is ?

@alexander-akait
Copy link
Member

I don't think it is possible to solve on plugin level without extra hacks

@paztis
Copy link
Author

paztis commented Feb 2, 2021

Will you accept PR for version 3.11, or you stop enhancement on it an onli work on v4 ?

@alexander-akait
Copy link
Member

If this does not require global changes, we can accept it, but you need send two PRs for v3 and v4

@paztis
Copy link
Author

paztis commented Feb 2, 2021

what it will require I think is a modification / enhancement of the injectClient init param signature, or another init param.
Is it considered as global ?

@alexander-akait
Copy link
Member

I think yes, for me it is not global changes

@paztis
Copy link
Author

paztis commented Feb 6, 2021

PR created in v3: #2995
Tell me if it is ok before I start master report

@paztis
Copy link
Author

paztis commented Feb 7, 2021

Here is the PR for master: #2998

@alexander-akait
Copy link
Member

I will look at this in this week

@paztis
Copy link
Author

paztis commented Feb 14, 2021

Have you found time to look at it ?

@alexander-akait
Copy link
Member

On the next week I am working on dev server, so I will look at near future

@paztis
Copy link
Author

paztis commented Mar 2, 2021

any news for the review ?

@alexander-akait
Copy link
Member

Still on my roadmap (near future)

@paztis
Copy link
Author

paztis commented Mar 10, 2023

any updated ? I open it 2 years ago now

@alexander-akait
Copy link
Member

@paztis Yeah, sorry for that, let's do rebase of you PR, I want to do a new release soon, so let's incude this in a new release

@alexander-akait
Copy link
Member

Anyway why don't use Function?

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

No branches or pull requests

2 participants