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

Disk cache + asset module regression in v5.72.0 #16160

Closed
mattlewis92 opened this issue Aug 17, 2022 · 27 comments · Fixed by #16703
Closed

Disk cache + asset module regression in v5.72.0 #16160

mattlewis92 opened this issue Aug 17, 2022 · 27 comments · Fixed by #16703

Comments

@mattlewis92
Copy link
Member

Bug report

What is the current behavior?

Webpack throws this error when using asset modules + disk caching:

Error: Cannot read properties of undefined (reading 'get')
during rendering of asset asset/resource|/Users/mattlewis/Code/clickup/frontend/node_modules/.pnpm/resolve-url-loader@5.0.0/node_modules/resolve-url-loader/index.js??ruleSet[1].rules[0].use[0]!/Users/mattlewis/Code/clickup/frontend/node_modules/.pnpm/sass-loader@13.0.2/node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[0].use[1]!/Users/mattlewis/Code/clickup/frontend/libs/theme/quill/quill-lazy-styles.scss?lazy-styles

The build works fine the first time, but then when making a change to the source and rebuilding it throws this error.

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

I'm really struggling to reproduce this outside of our app, it seems very nuanced and even the conditions to consistently reproduce are difficult.

After some debugging I determined the source of the regression was this change: #15515

The error is getting thrown here:

codeGenResult.data.get("filename"),
which is throwing because both module.buildInfo.filename and codeGenResult.data are undefined.

Adding some logs to that line, the value of codeGenResult is:

{
  sources: Map(1) {
    'asset' => CachedSource {
      _source: [Function],
      _cachedSourceType: false,
      _cachedSource: undefined,
      _cachedBuffer: <Buffer 2e 73 6c 61 73 68 2d 63 6f 6d 6d 61 6e 64 73 5f 6f 70 65 6e 20 2e 71 6c 2d 65 64 69 74 6f 72 20 2e 63 75 2d 73 6c 61 73 68 2d 63 6f 6d 6d 61 6e 64 5f ... 223147 more bytes>,
      _cachedSize: undefined,
      _cachedMaps: [Map],
      _cachedHashUpdate: [Array]
    }
  },
  runtimeRequirements: Set(0) {},
  data: undefined
}

and the value of module is:

NormalModule {
  dependencies: [],
  blocks: [],
  parent: undefined,
  type: 'asset/resource',
  context: '/Users/mattlewis/Code/clickup/frontend/libs/theme/quill',
  layer: null,
  needId: true,
  debugId: 2710,
  resolveOptions: {},
  factoryMeta: undefined,
  useSourceMap: false,
  useSimpleSourceMap: false,
  _warnings: undefined,
  _errors: undefined,
  buildMeta: { exportsType: 'default', defaultObject: false },
  buildInfo: {
    cacheable: true,
    parsed: true,
    fileDependencies: undefined,
    contextDependencies: undefined,
    missingDependencies: undefined,
    buildDependencies: LazySet {
      _set: [Set],
      _toMerge: Set(0) {},
      _toDeepMerge: [],
      _needMerge: false,
      _deopt: false
    },
    valueDependencies: undefined,
    hash: '8795b2198d6a3fc4',
    assets: undefined,
    assetsInfo: undefined,
    strict: true,
    dataUrl: false,
    snapshot: Snapshot {
      _flags: 5001,
      startTime: 1660738122934,
      fileTimestamps: undefined,
      fileHashes: undefined,
      fileTshs: [Map],
      contextTimestamps: undefined,
      contextHashes: undefined,
      contextTshs: undefined,
      missingExistence: [Map],
      managedItemInfo: [Map],
      managedFiles: [Set],
      managedContexts: undefined,
      managedMissing: undefined,
      children: [Set]
    }
  },
  presentationalDependencies: undefined,
  codeGenerationDependencies: undefined,
  request: '/Users/mattlewis/Code/clickup/frontend/node_modules/.pnpm/resolve-url-loader@5.0.0/node_modules/resolve-url-loader/index.js??ruleSet[1].rules[0].use[0]!/Users/mattlewis/Code/clickup/frontend/node_modules/.pnpm/sass-loader@13.0.2/node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[0].use[1]!/Users/mattlewis/Code/clickup/frontend/libs/theme/quill/quill-lazy-styles.scss?lazy-styles',
  userRequest: '/Users/mattlewis/Code/clickup/frontend/libs/theme/quill/quill-lazy-styles.scss?lazy-styles',
  rawRequest: '@cu/theme/quill/quill-lazy-styles.scss?lazy-styles',
  binary: true,
  parser: AssetParser { dataUrlCondition: false },
  parserOptions: undefined,
  generator: AssetGenerator {
    dataUrlOptions: undefined,
    filename: '[hash].css',
    publicPath: undefined,
    outputPath: undefined,
    emit: true
  },
  generatorOptions: { filename: '[hash].css' },
  resource: '/Users/mattlewis/Code/clickup/frontend/libs/theme/quill/quill-lazy-styles.scss?lazy-styles',
  resourceResolveData: {
    _ResolverCachePluginCacheMiss: true,
    context: {
      issuer: '/Users/mattlewis/Code/clickup/frontend/libs/common/lazy-link/src/lib/lazy-link.service.ts',
      issuerLayer: null,
      compiler: undefined
    },
    path: '/Users/mattlewis/Code/clickup/frontend/libs/theme/quill/quill-lazy-styles.scss',
    request: undefined,
    query: '?lazy-styles',
    fragment: '',
    module: false,
    directory: false,
    file: false,
    internal: false,
    fullySpecified: false,
    descriptionFilePath: '/Users/mattlewis/Code/clickup/frontend/package.json',
    descriptionFileData: {
      name: 'clickup-frontend',
      version: '0.0.0',
      license: 'UNLICENSED',
      scripts: [Object],
      'lint-staged': [Object],
      private: true,
      dependencies: [Object],
      devDependencies: [Object],
      optionalDependencies: [Object],
      prettier: [Object],
      'jest-junit': [Object],
      engines: [Object],
      pnpm: [Object],
      packageManager: 'pnpm@7.9.0'
    },
    descriptionFileRoot: '/Users/mattlewis/Code/clickup/frontend',
    relativePath: './libs/theme/quill/quill-lazy-styles.scss',
    typescriptPathMapped: true,
    __innerRequest_request: undefined,
    __innerRequest_relativePath: './libs/theme/quill/quill-lazy-styles.scss',
    __innerRequest: './libs/theme/quill/quill-lazy-styles.scss'
  },
  matchResource: undefined,
  loaders: [
    {
      loader: '/Users/mattlewis/Code/clickup/frontend/node_modules/.pnpm/resolve-url-loader@5.0.0/node_modules/resolve-url-loader/index.js',
      options: [Object],
      ident: 'ruleSet[1].rules[0].use[0]'
    },
    {
      loader: '/Users/mattlewis/Code/clickup/frontend/node_modules/.pnpm/sass-loader@13.0.2/node_modules/sass-loader/dist/cjs.js',
      options: [Object],
      ident: 'ruleSet[1].rules[0].use[1]'
    }
  ],
  error: null,
  _source: RawSource {
    _valueIsBuffer: true,
    _value: <Buffer 2e 73 6c 61 73 68 2d 63 6f 6d 6d 61 6e 64 73 5f 6f 70 65 6e 20 2e 71 6c 2d 65 64 69 74 6f 72 20 2e 63 75 2d 73 6c 61 73 68 2d 63 6f 6d 6d 61 6e 64 5f ... 223147 more bytes>,
    _valueAsBuffer: <Buffer 2e 73 6c 61 73 68 2d 63 6f 6d 6d 61 6e 64 73 5f 6f 70 65 6e 20 2e 71 6c 2d 65 64 69 74 6f 72 20 2e 63 75 2d 73 6c 61 73 68 2d 63 6f 6d 6d 61 6e 64 5f ... 223147 more bytes>,
    _valueAsString: undefined
  },
  _sourceSizes: undefined,
  _sourceTypes: Set(2) { 'javascript', 'asset' },
  _lastSuccessfulBuildMeta: { exportsType: 'default', defaultObject: false },
  _forceBuild: false,
  _isEvaluatingSideEffects: false,
  _addedSideEffectsBailout: undefined,
  _ast: null
}

The asset module rule in the webpack config we're using is:

{
    resourceQuery: /lazy-styles/,
    type: 'asset/resource',
    generator: {
      filename: '[hash].css',
    },
    test: /\.scss$/,
    use: [
      {
        loader: 'resolve-url-loader',
        options: {
          sourceMap: config.mode === 'development',
        },
      },
      {
        loader: 'sass-loader',
        options: {
          sourceMap: true,
        },
      },
    ],
  }

Maybe this info is enough to provide some clues as to what's going wrong, but also happy to provide more info or debug output!

What is the expected behavior?

It should not throw an error.

Other relevant information:
webpack version: 5.72.0
Node.js version: 16.16.0
Operating System: MacOS
Additional tools:

@alexander-akait
Copy link
Member

I think you have outdated cache, ideally you should remove cache when you update webpack, maybe we should do it on our side (i.e. store version in cache and invalidate it when version changed), when you remove old cache is it help?

@mattlewis92
Copy link
Member Author

I think you have outdated cache, ideally you should remove cache when you update webpack, maybe we should do it on our side (i.e. store version in cache and invalidate it when version changed), when you remove old cache is it help?

I deleted the cache after doing the webpack upgrade though, it was the same version of webpack (v5.72.0) that wrote the cache to disk

@alexander-akait
Copy link
Member

/cc @vankop

@ryanwilsonperkin
Copy link
Contributor

I've also noticed this issue affecting some of our builds that use a cache produced in CI and re-used on subsequent CI runs. For us, it seems to happen particularly when trying to use an SVG asset from the cache. We have approximately the same error message except the asset comes through the image minimizer plugin

ERROR in Cannot read properties of undefined (reading 'get')
--
  | during rendering of asset asset/resource\|/app/node_modules/image-minimizer-webpack-plugin/dist/loader.js??ruleSet[1].rules[9].use[0]!/app/path/to/images/balloons.svg
  | TypeError: Cannot read properties of undefined (reading 'get')
  | during rendering of asset asset/resource\|/app/node_modules/image-minimizer-webpack-plugin/dist/loader.js??ruleSet[1].rules[9].use[0]!/app/path/to/images/balloons.svg
  | at /app/node_modules/webpack/lib/asset/AssetModulesPlugin.js:186:30
  | at Hook.eval [as call] (eval at create (/app/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:17:16)
  | at Compilation.getRenderManifest (/app/node_modules/webpack/lib/Compilation.js:4513:36)
  | at /app/node_modules/webpack/lib/Compilation.js:4533:22
  | at symbolIterator (/app/node_modules/neo-async/async.js:3482:9)
  | at done (/app/node_modules/neo-async/async.js:3527:9)
  | at /app/node_modules/neo-async/async.js:2830:7
  | at done (/app/node_modules/neo-async/async.js:2865:11)
  | at /app/node_modules/neo-async/async.js:2818:7
  | at /app/node_modules/webpack/lib/Compilation.js:4658:10

@alexander-akait
Copy link
Member

alexander-akait commented Sep 14, 2022

@ryanwilsonperkin hm, we can have a bug in image-minimizer-webpack-plugin, can you try to create reproducible test repo?

@ryanwilsonperkin
Copy link
Contributor

Hoping to yes, I haven't been able to reproduce the bug yet unfortunately

@alexander-akait
Copy link
Member

https://github.com/webpack/webpack/blob/main/lib/asset/AssetModulesPlugin.js#L186, hm, filename was lost, what is the version of image-minimizer-webpack-plugin (do you have the latest version)? And can you provide options for this plugin?

@ryanwilsonperkin
Copy link
Contributor

Interestingly it doesn't seem like it's the filename that's getting lost, it's the data itself that's undefined based on my reading of the error Cannot read properties of undefined (reading 'get')

@ryanwilsonperkin
Copy link
Contributor

@alexander-akait managed to get a minimal reproduction here: https://github.com/ryanwilsonperkin/webpack-16160
This is just using the image minimizing plugin as well as the filesystem cache. Full reproduction steps in the readme of that repo, but I can now reliably reproduce this by rebuilding after making any change to the SVG asset that's being bundled. I suspect that the loader is using contenthash in one place and filename hash in another

@ryanwilsonperkin
Copy link
Contributor

Further discovery, by dropping a couple of console logs in the AssetModulesPlugin.js I was able to find that the data object is always undefined for this asset, it's just that when it's on the second build (after a modification) then the module.buildInfo.fullContentHash is also undefined, which causes it to fall through to trying to access the data.get method

@ryanwilsonperkin
Copy link
Contributor

Interestingly, if I install the imagemin-pngquant library and add "png" to the list of files that I want to pass through the minifier, it does not cause errors across re-builds, so this does seem to be specific to SVG files.

@alexander-akait
Copy link
Member

Thank you for the report, I will look at this soon, I think bug somewhere in image-minimizer-webpack-plugin

@alexander-akait
Copy link
Member

I found it buggy only when content of a file was changed in loader, looks like somewhere bug in webpack... continue investigation

@alexander-akait
Copy link
Member

alexander-akait commented Sep 16, 2022

Another intresting thing, if you will use console.log(new URL("./image.svg", import.meta.url)); - no problems (recommended, because import png from "./image.png" is not supporting without a bundler)

@alexander-akait
Copy link
Member

alexander-akait commented Sep 16, 2022

When we do code generation we store only (https://github.com/webpack/webpack/blob/main/lib/Compilation.js#L3326):

// `${module.identifier()}|${module.type}|${getRuntimeKey(runtime)}`:
// asset/resource|/path/to/webpack-16160/node_modules/image-minimizer-webpack-plugin/dist/loader.js??ruleSet[1].rules[0].use!/path/to/webpack-16160/src/image.svg|main

{
  sources: Map(1) {
    'asset' => CachedSource {
      _source: [Function],
      _cachedSourceType: false,
      _cachedSource: undefined,
      _cachedBuffer: <Buffer 3c 73 76 67 20 78 6d 6c 6e 73 3d 22 68 74 74 70 3a 2f 2f 77 77 77 2e 77 33 2e 6f 72 67 2f 32 30 30 30 2f 73 76 67 22 20 78 6d 6c 6e 73 3a 78 6c 69 6e ... 1591 more bytes>,
      _cachedSize: undefined,
      _cachedMaps: Map(0) {},
      _cachedHashUpdate: undefined
    }
  },
  runtimeRequirements: Set(0) {},
  data: undefined
}

I.e. no javascript code generation in sources (like we don't generate JS file for the asset module), but we do javascript code generation and store it for index.js|48527077921e8eaca24c917175b660a5|main (i.e. for JS module where we have import ... from ... with the asset module, not for the asset module itself). If we change import ... from ... to new URL(...);, all works fine and we generate asset and javascript sources for the asset module.

With new URL(...) you will not have these problem (and as I written above it is valid usage and can work without bundler)

Not sure why we do it, need advice from @sokra

Also if we change order here https://github.com/webpack/webpack/blob/main/lib/ChunkGraph.js#L620, i.e. module.getSourceTypes() || this._getOverwrittenModuleSourceTypes(module) we fix the problem too

Also we can alway

@ryanwilsonperkin
Copy link
Contributor

(recommended, because import png from "./image.png" is not supporting without a bundler)

With new URL(...) you will not have these problem (and as I written above it is valid usage and can work without bundler)

While I appreciate that this might give us the flexibility of not being bound to the bundler, we're already heavily invested in using the bundler for features like this and wouldn't be likely to change all of our image references to URL-based approaches. It seems like that wouldn't work for asset/inline, or if assets are configured with a different output directory.

@alexander-akait
Copy link
Member

It seems like that wouldn't work for asset/inline, or if assets are configured with a different output directory.

Can you clarify?

Anyway I marked it as a bug, because we should fix it

@ryanwilsonperkin
Copy link
Contributor

In the case of asset/inline the bundler would take the asset and inline it into the source as a data URI. If we were using new URL("./image.svg", import.meta.url) then i don't think that would work correctly because it would be trying to treat that inlined URI as a relative url with path name. Similar issue for asset/source

Anyway I marked it as a bug, because we should fix it

Thanks! I'm also looking into it, although I'm new to the webpack source code

@alexander-akait
Copy link
Member

If we were using new URL("./image.svg", import.meta.url) then i don't think that would work correctly because it would be trying to treat that inlined URI as a relative url with path name.

No, try new URL("data:image/gif;base64,R0lGODdhMAAwAPAAAAAAAP///ywAAAAAMAAwAAAC8IyPqcvt3wCcDkiLc7C0qwyGHhSWpjQu5yqmCYsapyuvUUlvONmOZtfzgFzByTB10QgxOR0TqBQejhRNzOfkVJ+5YiUqrXF5Y5lKh/DeuNcP5yLWGsEbtLiOSpa/TPg7JpJHxyendzWTBfX0cxOnKPjgBzi4diinWGdkF8kjdfnycQZXZeYGejmJlZeGl9i2icVqaNVailT6F5iJ90m6mvuTS4OK05M0vDk0Q4XUtwvKOzrcd3iq9uisF81M1OIcR7lEewwcLp7tuNNkM3uNna3F2JQFo97Vriy/Xl4/f1cf5VWzXyym7PHhhx4dbgYKAAA7", import.meta.url), webpack doesn the same for inline

@9-lives
Copy link

9-lives commented Dec 14, 2022

What if i bundled images in a library with rollup? I just want to put all of images on CDN. But it seems that part of image's url is not replaced to correct publicpath by using webpack.

@alexander-akait
Copy link
Member

You can set the public path at runtime or via config

@9-lives
Copy link

9-lives commented Dec 14, 2022

You can set the public path at runtime or via config
// library A is bundled by rollup
//
// library A p.js
new URL('../assets/p.jpg', import.meta.url)

// library A dist
new URL('../assets/p.jpg', import.meta.url)

Sorry to bother you again. But when i bundled my application with webpack, I just can choose preserve "import.meta.url" or resolve it. How can i make publicPath replace it correctly?

@alexander-akait
Copy link
Member

alexander-akait commented Dec 14, 2022

We have the special thing - https://webpack.js.org/guides/public-path/#on-the-fly, it should work, try it

@9-lives
Copy link

9-lives commented Dec 22, 2022

It's not just the url matter. Webpack will not output library A's assets to dist directory if there is no import statement. Thus, library A's assets is missing and uploading it to CDN is obviously a mistake.

@7rulnik
Copy link
Contributor

7rulnik commented Jan 19, 2023

Looks like I faced same bug

I have import

import airplane from './assets/airplane_gradient.svg';

Loader for svg looks like this:

      {
        test: /\.(png|jpg|gif|svg|webp|jp2|jxr|eot|ttf|woff|woff2|mp4|webm)$/,
        type: 'asset/resource',
        resourceQuery: /^(?!\?react-svg$).*/,
      }

and issue looks like this

ERROR in [entry] [initial]

Cannot read properties of undefined (reading 'get')

during rendering of asset asset/resource|/home/jenkins/agent/workspace/selene/node_modules/@aviasales/ui/shared/more_gradient_icons/assets/airplane_gradient.svg

@ryanwilsonperkin
Copy link
Contributor

@alexander-akait I've been able to isolate this further and I believe this is a bug in Webpack and not in the image-minimizer-webpack-plugin

I've updated my example repo to replace that plugin with a simple loader that just does the following:

// Naive loader that just returns a constant static SVG placeholder
module.exports = function loader(content) {
  return '<svg></svg>';
}

Using this, we still get the same error message after building, modifying the svg file, and then building again

@alexander-akait
Copy link
Member

@ryanwilsonperkin Thank you for your PR, we will review soon

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

Successfully merging a pull request may close this issue.

5 participants