gregberge / loadable-components

The recommended Code Splitting library for React ✂️✨
https://loadable-components.com
MIT License
7.65k stars 380 forks source link

Support multiple extractors #365

Closed 7rulnik closed 4 years ago

7rulnik commented 5 years ago

🚀 Feature Proposal

Possibility to pass multiple extractors to ChunkExtractorManager

Motivation

It could be useful when you ship two bundles: for modern and old browsers. Currently we can achieve this by overriding stats field in ChunkExtractor instance, but it's a bit dirty solution.

Example

<ChunkExtractorManager extractor={[firstExtractor, secondExtractor ]}>
gregberge commented 5 years ago

Yes we have to support it. It is for me the next priority for Loadable Components. I don't have so much time to work on it, but I will try to add it as soon as possible!

7rulnik commented 5 years ago

@neoziro I can send PR if you don't mind

gregberge commented 5 years ago

I am not sure about the API, so I prefer to do it by myself.

aselhid commented 5 years ago

any progress on this feature yet?

Edit : mjs extension support would be nice because bundler like webpack already support that extension since version 4

7rulnik commented 5 years ago

@aselhid webpack doesn't have support of output .mjs right now. Webpack will support it in feature versions. And it's not related to this issue

7rulnik commented 4 years ago

So if someone is looking for workaround so here you go:

Firstly we need to patch ChunkExtractor

// https://github.com/smooth-code/loadable-components/issues/365

import { ChunkExtractor } from '@loadable/server';

function getSriHtmlAttributes(asset) {
    if (!asset.integrity) {
        return '';
    }

    return ` integrity="${asset.integrity}"`;
}

function handleExtraProps(asset, extraProps) {
    return typeof extraProps === 'function' ? extraProps(asset) : extraProps;
}

function extraPropsToString(asset, extraProps) {
    return Object.entries(handleExtraProps(asset, extraProps)).reduce(
        (acc, [key, value]) => (key === 'nomodule' ? `${acc} ${key}` : `${acc} ${key}="${value}"`),
        ''
    );
}

function assetToScriptTag(asset, extraProps) {
    return `<script async data-chunk="${asset.chunk}" src="${asset.url}"${getSriHtmlAttributes(
        asset
    )}${extraPropsToString(asset, extraProps)}></script>`;
}

export class PatchedChunkExtractor extends ChunkExtractor {
    getScriptTags(extraProps = {}) {
        const mainAssets = this.getMainAssets('script');
        const assetsScriptTags = mainAssets.map(asset => assetToScriptTag(asset, extraProps));

        return assetsScriptTags.join('\n');
    }
}

Also we need a map function for extractor and multiple stats (but it's optional):

/* eslint-disable no-param-reassign */
export function mapExtractorWithStats(extractor, statsAsArr, mappper) {
    const originalStats = extractor.stats;
    const originalPublicPath = extractor.publicPath;
    const originalOutputPath = extractor.outputPath;

    const arrs = statsAsArr.map((stat, index) => {
        extractor.stats = stat;
        extractor.publicPath = originalPublicPath || stat.publicPath;
        extractor.outputPath = originalPublicPath || stat.outputPath;

        return mappper(extractor, index);
    });

    extractor.stats = originalStats;
    extractor.publicPath = originalPublicPath;
    extractor.outputPath = originalOutputPath;

    return arrs;
}

And now we can generate markup:

const extractor = new PatchedChunkExtractor({
  stats: {},
  publicPath: process.env.PUBLIC_PATH
});

const jsx = extractor.collectChunks(<App />);

const markup = renderToString(jsx);

const [legacyResult, modernResult] = mapExtractorWithStats(
  extractor,
  [statsLegacy, statsModern],
  (patchedExtractor, index) => {
    const isModern = index === 1;
    const scriptOpts = {};

    if (isModern) {
      scriptOpts.type = "module";
    } else {
      scriptOpts.nomodule = true;
    }

    const requiredChunksScriptTag = patchedExtractor.getRequiredChunksScriptTag({});
    const scriptTags = patchedExtractor.getScriptTags(scriptOpts);
    const styleTags = patchedExtractor.getStyleTags();

    return {
      requiredChunksScriptTag,
      scriptTags,
      styleTags
    };
  }
);

/* Please read https://github.com/smooth-code/loadable-components/issues/365#issuecomment-536486639 before using it */
const html = `<html>
  <head>${linkTags}</head>
  <body>
    <div id="root">${markup}</div>
    <!-- required chunks script tag -->
    ${modernResult.requiredChunksScriptTag}

    <!-- modern assets -->
    ${modernResult.scriptTags}

    <!-- legacy assets -->
    ${legacyResult.scriptTags}
  </body>
</html>`
theKashey commented 4 years ago

Using modern/nomodern is not a good idea for non-modern browsers. There are still some issues for Safari, and a BIG ONE with Edge. Plus - without modulepreload it's not as efficient as it could be.

7rulnik commented 4 years ago

@theKashey I use https://babeljs.io/docs/en/babel-preset-env#targetsesmodules and ship it via type="module"/nomodule. Seems that it's working for me. What issues that you're talking about? But also I believe that it's kinda offtopic.

theKashey commented 4 years ago

Check https://github.com/johnstew/differential-serving about how module/nomodule support, and Enabling Modern JavaScript on npm for the most modern "workarounds". This is not offtopic - I urge to read information behind the links and update your example with less dangerous solution before someone would blindly pick it.

7rulnik commented 4 years ago

@theKashey just checked differential-serving link. Thank you. Did not know about the issue with edge@18. I think that we should add a link to it on babel-preset-env page.

Also, there are some problems with Safari@10 like this one. So maybe it's better to use UA-based solution.

And also I added link to your comment into my example

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

VitaliiB commented 4 years ago

+1 for the feature. Meanwhile, i took the following workaround, seems to work for me: https://github.com/gregberge/loadable-components/pull/446#issuecomment-549366549

kevinfarrugia commented 2 years ago

What is the current recommendation if you want to implement the module / nomodule pattern?