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

Critical dependency: the request of a dependency is an expression #4091

Closed
13 tasks
muuvmuuv opened this issue Feb 3, 2023 · 15 comments
Closed
13 tasks

Critical dependency: the request of a dependency is an expression #4091

muuvmuuv opened this issue Feb 3, 2023 · 15 comments

Comments

@muuvmuuv
Copy link

muuvmuuv commented Feb 3, 2023

Description

In Angular with the latest 3.x release, I am getting this warning from Webpack.

./node_modules/.pnpm/@google+model-viewer@3.0.0/node_modules/@google/model-viewer/lib/three-components/TextureUtils.js:62:18-47 - Warning: Critical dependency: the request of a dependency is an expression

Live Demo

https://glitch.com/edit/#!/model-viewer

I don't think one is needed, it's just the way the dep is imported.
https://stackoverflow.com/questions/42908116/webpack-critical-dependency-the-request-of-a-dependency-is-an-expression

Version

  • model-viewer: v3.X.X

Browser Affected

  • Chrome, version: xx.x.xxxx.xx
  • Edge
  • Firefox
  • IE
  • Safari

OS

  • Android
  • iOS
  • Linux
  • MacOS
  • Windows

AR

  • WebXR
  • SceneViewer
  • QuickLook
@elalish
Copy link
Collaborator

elalish commented Feb 3, 2023

I would guess this is related to #4090? The basic issue is that the library to load lottie animations is as large as model-viewer itself, so we definitely don't want to pack it into our bundle. I designed it to lazy-load as needed just like the DRACO decoder. However, it sounds like something I did is confusing some of the tooling systems - help would be much appreciated. Does anyone know why it complains for Lottie but not for DRACO?

@muuvmuuv
Copy link
Author

muuvmuuv commented Feb 4, 2023

Maybe I can setup a repl with webpack but I guess it will alteady be resolved if added as optional/peerDependency. And yes its related so we can close this one if you want?

@elalish
Copy link
Collaborator

elalish commented Feb 6, 2023

Okay, sounds good.

@elalish elalish closed this as completed Feb 6, 2023
@elalish elalish mentioned this issue Feb 7, 2023
12 tasks
@muuvmuuv
Copy link
Author

So, #4090 is fixed, but now I need to explicitly install three otherwise the build fails and even if installed I still get this Webpack warning… :(

@marcogianni
Copy link

marcogianni commented Feb 23, 2023

I also have the same warning message with three already installed ( Next.js: 13.1.6 ).

./node_modules/@google/model-viewer/lib/three-components/TextureUtils.js
Critical dependency: the request of a dependency is an expression

Import trace for requested module:
./node_modules/@google/model-viewer/lib/three-components/TextureUtils.js
./node_modules/@google/model-viewer/lib/three-components/Renderer.js
./node_modules/@google/model-viewer/lib/features/loading.js
./node_modules/@google/model-viewer/lib/model-viewer.js
./components/3d-assets/SingleAssetImage.js

@elalish
Copy link
Collaborator

elalish commented Feb 23, 2023

Well, at least it's only a warning! Maybe we're looking for this? https://stackoverflow.com/a/73359606 I don't quite understand what difference it makes. In any case, you do not want lottie-web bundled in, unless you're actually using it - it's quite large. This is why we load it lazily. Is this blocking anyone? I'd prefer not to make further changes here until someone who understands Webpack tells me the right thing to do and tests it.

@muuvmuuv
Copy link
Author

Also found this https://webpack.js.org/api/module-methods/

Bildschirm­foto 2023-02-27 um 08 55 29

I have confirmed that this would remove the warning but idk if this breaks other functionallity. Further reading: webpack/webpack#7713 (comment)

@muuvmuuv
Copy link
Author

Seems to be the right thing after reading this one here: webpack/webpack#15957

Webpack will always try to import it/or node.

@muuvmuuv
Copy link
Author

muuvmuuv commented Mar 9, 2023

@elalish just found that NX is doing it the same way :) so guess its fine model-viewer adapts it: https://github.com/nrwl/nx/blob/master/packages/angular/mf/mf.ts#L41

@syui
Copy link

syui commented Sep 6, 2023

I get an error in vue.
By the way, the build succeeds on local (mac-m1).

name: github pages

on:
  push:
    branches:
    - main

jobs:
  build-deploy:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v1
    - uses: actions/setup-node@v1
      with:
        node-version: 14
        ref: main
        submodules: true
        fetch-depth: 0
    - run: |
           yarn install
           yarn add @google/model-viewer

    - name: Build
      env: 
        TZ: "Asia/Tokyo"
      run: |
           yarn build

src/App.vue

computed: {
			loadComponent() {
				return () => import('@google/model-viewer');
			}
		},
"node_modules/@google/model-viewer": {
      "version": "3.2.1",
      "license": "Apache-2.0",
      "dependencies": {
        "lit": "^2.7.2"
      },
      "engines": {
        "node": ">=6.0.0"
      },
      "peerDependencies": {
        "three": "^0.154.0"
      }
    },

@elalish
Copy link
Collaborator

elalish commented Sep 7, 2023

What error?

@syui
Copy link

syui commented Sep 7, 2023

 ERROR  Failed to compile with 18 errors1:18:19 AM

These dependencies were not found:

* three in ./node_modules/@google/model-viewer/lib/model-viewer.js, ./node_modules/@google/model-viewer/lib/features/annotation.js and 3 others
* three/examples/jsm/exporters/GLTFExporter.js in ./node_modules/@google/model-viewer/lib/features/scene-graph.js
* three/examples/jsm/exporters/USDZExporter.js in ./node_modules/@google/model-viewer/lib/features/ar.js
* three/examples/jsm/loaders/DRACOLoader.js in ./node_modules/@google/model-viewer/lib/three-components/CachingGLTFLoader.js
* three/examples/jsm/loaders/GLTFLoader.js in ./node_modules/@google/model-viewer/lib/three-components/CachingGLTFLoader.js
* three/examples/jsm/loaders/KTX2Loader.js in ./node_modules/@google/model-viewer/lib/three-components/CachingGLTFLoader.js
* three/examples/jsm/loaders/RGBELoader.js in ./node_modules/@google/model-viewer/lib/three-components/TextureUtils.js
 ERROR  Build failed with errors.
* three/examples/jsm/renderers/CSS2DRenderer.js in ./node_modules/@google/model-viewer/lib/three-components/Hotspot.js
* three/examples/jsm/shaders/HorizontalBlurShader.js in ./node_modules/@google/model-viewer/lib/three-components/Shadow.js
* three/examples/jsm/shaders/VerticalBlurShader.js in ./node_modules/@google/model-viewer/lib/three-components/Shadow.js
* three/examples/jsm/utils/SceneUtils.js in ./node_modules/@google/model-viewer/lib/three-components/ModelScene.js
* three/examples/jsm/utils/SkeletonUtils.js in ./node_modules/@google/model-viewer/lib/three-components/GLTFInstance.js
* three/examples/jsm/webxr/XREstimatedLight.js in ./node_modules/@google/model-viewer/lib/three-components/ARRenderer.js
* three/src/math/MathUtils.js in ./node_modules/@google/model-viewer/lib/three-components/Shadow.js

To install them, you can run: npm install --save three three/examples/jsm/exporters/GLTFExporter.js three/examples/jsm/exporters/USDZExporter.js three/examples/jsm/loaders/DRACOLoader.js three/examples/jsm/loaders/GLTFLoader.js three/examples/jsm/loaders/KTX2Loader.js three/examples/jsm/loaders/RGBELoader.js three/examples/jsm/renderers/CSS2DRenderer.js three/examples/jsm/shaders/HorizontalBlurShader.js three/examples/jsm/shaders/VerticalBlurShader.js three/examples/jsm/utils/SceneUtils.js three/examples/jsm/utils/SkeletonUtils.js three/examples/jsm/webxr/XREstimatedLight.js three/src/math/MathUtils.js
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.```

@elalish
Copy link
Collaborator

elalish commented Sep 7, 2023

I don't know yarn, but I'd guess you need something like yarn add three to get the peer dependency.

@syui
Copy link

syui commented Sep 8, 2023

Thanks, gh-actions came through.

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

4 participants