feat: make Webpack url-loader limit configurable (env variable) #5498

stnor · 2021-09-07T14:36:22Z

Motivation

Fixes #5493

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Run yarn run start:website should work as before
Modify package.jsonand change yarn run start:website to cross-env URL_LOADER_LIMIT=999999 yarn workspace website start. The new limit should be in effect. I tested this by printing out the value of the const in the affected file.

…nment variable 'URL_LOADER_LIMIT'.

RDIL

LGTM as a temporary solution. I think this is a good change for the time being.

netlify · 2021-09-07T14:43:29Z

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 3743375

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/614a3b75cb81270007a2013e

😎 Browse the preview: https://deploy-preview-5498--docusaurus-2.netlify.app

github-actions · 2021-09-07T14:43:43Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟢 Performance	96
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

Lighthouse ran on https://deploy-preview-5498--docusaurus-2.netlify.app/

packages/docusaurus/src/webpack/utils.ts

Co-authored-by: Joshua Chen <[email protected]>

slorber · 2021-09-21T17:34:20Z

Thanks, agree this is a good-enough temporary change.

file-loader and url-loader are going to be deprecated someday and we'll migrate to the new asset system (POC #4708) so we'll avoid documenting this on purpose because the API surface is not ideal and we'll likely do a breaking change.

slorber · 2021-09-21T17:35:35Z

packages/docusaurus/src/webpack/utils.ts

-  const urlLoaderLimit = 10000;
+  // files/images < 10kb (overridable via 'URL_LOADER_LIMIT' environment
+  // variable) will be inlined as base64 strings directly in the html
+  const urlLoaderLimit = process.env.URL_LOADER_LIMIT ?? 10000;


Have you tested this locally? Afaik process.env.URL_LOADER_LIMIT would be a string, shouldn't it be converted to a number?

I also suggest:

moving it to packages/docusaurus/src/constants.ts

name it WEBPACK_ URL_LOADER_LIMIT

add comment to say it's temporary, link to this PR/issue

We'd rather centralize all those undocumented env variables in a single place

Have you tested this locally? Afaik process.env.URL_LOADER_LIMIT would be a string, shouldn't it be converted to a number?

The loader accepts number | string.

I'll move the const and rename the ENV VAR as per your suggestion.

slorber · 2021-09-21T17:37:52Z

packages/docusaurus/src/webpack/utils.ts

-  const urlLoaderLimit = 10000;
+  // files/images < 10kb (overridable via 'URL_LOADER_LIMIT' environment
+  // variable) will be inlined as base64 strings directly in the html
+  const urlLoaderLimit = process.env.URL_LOADER_LIMIT ?? 10000;


I also suggest:

moving it to packages/docusaurus/src/constants.ts

name it WEBPACK_ URL_LOADER_LIMIT

add comment to say it's temporary, link to this PR/issue

We'd rather centralize all those undocumented env variables in a single place

* moving it to packages/docusaurus/src/constants.ts * name it WEBPACK_ URL_LOADER_LIMIT * add comment to say it's temporary, link to this PR/issue

slorber · 2021-09-22T09:46:25Z

Thanks

So it does work fine without using parseInt() on the env variable right?

slorber · 2021-09-22T09:50:48Z

Looks like it accepts a string indeed:

https://v4.webpack.js.org/loaders/url-loader/#options

johnnyreilly · 2023-01-04T06:04:28Z

I stumbled upon this when I was writing up how to use the
docusaurus-image-cloudinary-remark-plugin:

https://johnnyreilly.com/docusaurus-image-cloudinary-rehype-plugin#introducing-remark-cloudinary-docusaurus

The workaround works... but it's not great developer experience. @slorber would you be open to having this configured more directly somehow? Perhaps using the docusaurus.config.js?

slorber · 2023-01-04T16:09:11Z

@johnnyreilly what would be the api so that it's not overwhelming for 99% users not needing this option?

johnnyreilly · 2023-01-05T07:42:42Z

So @slorber my thought is that this would be optional - you only use it if you need to use it; by default you don't. Most folks wouldn't use this.

I don't have particular thoughts on what the API should be but here's an idea (you may not like it, but it could start the ball rolling). The configuration we want to tweak lives in this file:

https://github.com/facebook/docusaurus/blob/main/packages/docusaurus-utils/src/constants.ts

The existing constants.ts file becomes something like this:

export function makeConstants(configureConstants?: (constants: typeof CONSTANTS) => typeof CONSTANTS) {
  const CONSTANTS = {

    /** Node major version, directly read from env. */
    NODE_MAJOR_VERSION: parseInt(
      process.versions.node.split('.')[0]!,
      10,
    ),
    /** Node minor version, directly read from env. */
    NODE_MINOR_VERSION: parseInt(
      process.versions.node.split('.')[1]!,
      10,
    ),

    /** Docusaurus core version. */
    DOCUSAURUS_VERSION:
      // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires
      (require('../package.json') as { version: string }).version,

    /**
     * Can be overridden with cli option `--out-dir`. Code should generally use
     * `context.outDir` instead (which is always absolute and localized).
     */
    DEFAULT_BUILD_DIR_NAME: 'build',

    /**
     * Can be overridden with cli option `--config`. Code should generally use
     * `context.siteConfigPath` instead (which is always absolute).
     *
     * This does not have extensions, so that we can substitute different ones
     * when resolving the path.
     */
    DEFAULT_CONFIG_FILE_NAME: 'docusaurus.config',

    /** Can be absolute or relative to site directory. */
    BABEL_CONFIG_FILE_NAME:
      process.env.DOCUSAURUS_BABEL_CONFIG_FILE_NAME ?? 'babel.config.js',

    /**
     * Can be absolute or relative to site directory. Code should generally use
     * `context.generatedFilesDir` instead (which is always absolute).
     */
    GENERATED_FILES_DIR_NAME:
      process.env.DOCUSAURUS_GENERATED_FILES_DIR_NAME ?? '.docusaurus',

    /**
     * We would assume all of the site's JS code lives in here and not outside.
     * Relative to the site directory.
     */
    SRC_DIR_NAME: 'src',

    /**
     * Can be overridden with `config.staticDirectories`. Code should use
     * `context.siteConfig.staticDirectories` instead (which is always absolute).
     */
    DEFAULT_STATIC_DIR_NAME: 'static',

    /**
     * Files here are handled by webpack, hashed (can be cached aggressively).
     * Relative to the build output folder.
     */
    OUTPUT_STATIC_ASSETS_DIR_NAME: 'assets',

    /**
     * Components in this directory will receive the `@theme` alias and be able to
     * shadow default theme components.
     */
    THEME_PATH: `${SRC_DIR_NAME}/theme`,

    /**
     * All translation-related data live here, relative to site directory. Content
     * will be namespaced by locale.
     */
    DEFAULT_I18N_DIR_NAME: 'i18n',

    /**
     * Translations for React code.
     */
    CODE_TRANSLATIONS_FILE_NAME: 'code.json',

    /** Dev server opens on this port by default. */
    DEFAULT_PORT: 3000,

    /** Default plugin ID. */
    DEFAULT_PLUGIN_ID: 'default',

    /**
     * Allow overriding the limit after which the url loader will no longer inline
     * assets.
     *
     * @see https://github.com/facebook/docusaurus/issues/5493
     */
    WEBPACK_URL_LOADER_LIMIT:
      process.env.WEBPACK_URL_LOADER_LIMIT ?? 10000,

  }

  return (configureConstants) ? configureConstants(CONSTANTS) : CONSTANTS;
}

Imagine a docusaurus option called configureConstants; a function that takes the output of constants and allows users to fiddle with them to their hearts content.

It would be a power user feature to be sure - most people wouldn't use this.

What do you think?

Make urlLoaderLimit in the webpack config user-overridable via enviro…

5b4503e

…nment variable 'URL_LOADER_LIMIT'.

stnor requested review from lex111 and slorber as code owners September 7, 2021 14:36

facebook-github-bot added the CLA Signed Signed Facebook CLA label Sep 7, 2021

RDIL approved these changes Sep 7, 2021

View reviewed changes

Josh-Cena reviewed Sep 7, 2021

View reviewed changes

packages/docusaurus/src/webpack/utils.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

6f98006

Co-authored-by: Joshua Chen <[email protected]>

stnor mentioned this pull request Sep 8, 2021

Expose settings to control the minification process(es) #5493

Closed

slorber reviewed Sep 21, 2021

View reviewed changes

slorber requested changes Sep 21, 2021

View reviewed changes

stnor and others added 2 commits September 21, 2021 22:06

Changes as per @slorber's suggestions:

26fead7

* moving it to packages/docusaurus/src/constants.ts * name it WEBPACK_ URL_LOADER_LIMIT * add comment to say it's temporary, link to this PR/issue

Merge branch 'facebook:main' into url-loader-limit

3743375

slorber changed the title ~~Make urlLoaderLimit in the webpack config user-overridable via enviro…~~ feat: make Webpack url-loader limit configurable (env variable) Sep 22, 2021

slorber added the pr: new feature This PR adds a new API or behavior. label Sep 22, 2021

slorber merged commit dc5ec32 into facebook:main Sep 22, 2021

stnor deleted the url-loader-limit branch October 19, 2021 09:44

stnor restored the url-loader-limit branch October 19, 2021 09:44

stnor deleted the url-loader-limit branch October 19, 2021 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make Webpack url-loader limit configurable (env variable) #5498

feat: make Webpack url-loader limit configurable (env variable) #5498

stnor commented Sep 7, 2021 •

edited

Loading

RDIL left a comment

netlify bot commented Sep 7, 2021 •

edited

Loading

github-actions bot commented Sep 7, 2021 •

edited

Loading

slorber commented Sep 21, 2021

slorber Sep 21, 2021

slorber Sep 21, 2021

stnor Sep 21, 2021 •

edited

Loading

stnor Sep 21, 2021

slorber Sep 21, 2021

slorber commented Sep 22, 2021

slorber commented Sep 22, 2021

johnnyreilly commented Jan 4, 2023 •

edited

Loading

slorber commented Jan 4, 2023

johnnyreilly commented Jan 5, 2023 •

edited

Loading

feat: make Webpack url-loader limit configurable (env variable) #5498

feat: make Webpack url-loader limit configurable (env variable) #5498

Conversation

stnor commented Sep 7, 2021 • edited Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

RDIL left a comment

Choose a reason for hiding this comment

netlify bot commented Sep 7, 2021 • edited Loading

github-actions bot commented Sep 7, 2021 • edited Loading

slorber commented Sep 21, 2021

slorber Sep 21, 2021

Choose a reason for hiding this comment

slorber Sep 21, 2021

Choose a reason for hiding this comment

stnor Sep 21, 2021 • edited Loading

Choose a reason for hiding this comment

stnor Sep 21, 2021

Choose a reason for hiding this comment

slorber Sep 21, 2021

Choose a reason for hiding this comment

slorber commented Sep 22, 2021

slorber commented Sep 22, 2021

johnnyreilly commented Jan 4, 2023 • edited Loading

slorber commented Jan 4, 2023

johnnyreilly commented Jan 5, 2023 • edited Loading

stnor commented Sep 7, 2021 •

edited

Loading

netlify bot commented Sep 7, 2021 •

edited

Loading

github-actions bot commented Sep 7, 2021 •

edited

Loading

stnor Sep 21, 2021 •

edited

Loading

johnnyreilly commented Jan 4, 2023 •

edited

Loading

johnnyreilly commented Jan 5, 2023 •

edited

Loading