embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
339 stars 137 forks source link

`Outdated Optimize Dep` error when consuming 'normal' (direct) npm packages with vite – deps not ending up in `node_modules/.vite/deps` #2168

Open johanrd opened 2 days ago

johanrd commented 2 days ago

Steps to reproduce

1) Install a normal npm library, or clone this repositiory for reproduction: https://github.com/johanrd/ember-vite-npm

Detailed steps 1.1) create a new ember vite app with @NullVoxPopuli's great little ember-vite-blueprint `npx ember-cli@latest new glimmer-apollo-vite --blueprint @nullvoxpopuli/ember-vite-app --pnpm --typescript` 1.2) pnpm install any npm library, e.g. 'pnpm install sanitize-html` 1.3) import the pnpm library into wherever, e.g. application.gts 1.4) run `pnpm start`

2) See that the console errors with Failed to load resource: the server responded with a status of 504 (Outdated Optimize Dep)

Screenshot 2024-11-12 at 09 07 09

Npm packages most often do not end up in node_modules/.vite/deps. Possibly related to https://github.com/embroider-build/embroider/issues/1583.

Is there a config I am missing? I have tried to add include in optimizeDeps(), but that stopped the vite server completely.

patricklx commented 2 days ago

can you try running the command vite optimize? it is failing when I test it on your repo. But maybe that is related to my node_modules.

johanrd commented 2 days ago

if i run npx vite optimize I get:

Hash is consistent. Skipping. Use --force to override.

nothing added/ no changes to node_modules/.vite/deps

If i run npx vite optimize --force I get:

Forced re-optimization of dependencies
(node:7259) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Optimizing dependencies:
  @ember-data/debug/_app_/data-adapter, @ember/test-helpers, @embroider/config-meta-loader, @glimmer/component/_app_/component-managers/glimmer.js, ember-data/app/initializers/ember-data, ember-data/app/services/store, ember-data/app/transforms/boolean, ember-data/app/transforms/date, ember-data/app/transforms/number, ember-data/app/transforms/string, ember-load-initializers, ember-page-title, ember-page-title/_app_/services/page-title.js, ember-qunit, ember-resolver, ember-route-template, ember-source/@ember/application/index.js, ember-source/@ember/component/index.js, ember-source/@ember/component/template-only.js, ember-source/@ember/routing/router.js, ember-source/@ember/template-factory/index.js, ember-source/ember-testing/index.js, ember-source/ember/index.js, ember-welcome-page, qunit, qunit-dom, sanitize-html
✘ [ERROR] Could not read from file: ~/ember-vite-npm/node_modules/.pnpm/postcss@8.4.49/node_modules/postcss/lib/terminal-highlight

    node_modules/.pnpm/postcss@8.4.49/node_modules/postcss/lib/css-syntax-error.js:4:32:
      4 │ let terminalHighlight = require('./terminal-highlight');
        ╵                                 ~~~~~~~~~~~~~~~~~~~~~~

error when optimizing deps:
Error: Build failed with 1 error:
node_modules/.pnpm/postcss@8.4.49/node_modules/postcss/lib/css-syntax-error.js:4:32: ERROR: Could not read from file: ~/ember-vite-npm/node_modules/.pnpm/postcss@8.4.49/node_modules/postcss/lib/terminal-highlight
    at failureErrorWithLog (~/ember-vite-npm/node_modules/.pnpm/esbuild@0.21.5/node_modules/esbuild/lib/main.js:1472:15)
    at ~/ember-vite-npm/node_modules/.pnpm/esbuild@0.21.5/node_modules/esbuild/lib/main.js:945:25
    at ~/ember-vite-npm/node_modules/.pnpm/esbuild@0.21.5/node_modules/esbuild/lib/main.js:1353:9
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
patricklx commented 2 days ago

right, thats what i am seeing as well let terminalHighlight = require('./terminal-highlight'); but why is it importing postcss into the app itself?

patricklx commented 2 days ago

oh, its used by sanitize-html. just tried it on a repo without embroider and there the optimization of sanitize-html works.

they do have some funny exports, but should work regardless. https://github.com/postcss/postcss/blob/main/package.json#L36

mansona commented 2 days ago

I've had to fix this with sanatize-html before too. Unfortunately this isn't an embroider problem, I imagine you would have the exact same issue if you used it in a vanilla vite package too

I've fixed this in the past by patching postcss to add the extensions that it expects to be able to resolve that file: https://github.com/ember-learn/ember-api-docs/pull/920/files#diff-218116475a824ffaa3d495332d9bcf7dcd105c41f131ed71626cf1e7a66ac7d8

patricklx commented 2 days ago

@mansona it works in a vanilla vite app. and it works if i add if (path.includes('terminal-')) { return null; } to all build.onResolve in esbuild-resolver.js

patricklx commented 2 days ago

I think embroider needs to handle the browser field. Or somehow let vite deps plugin handle it.

it is marked as excluded here: https://github.com/postcss/postcss/blob/main/package.json#L132

https://github.com/defunctzombie/package-browser-field-spec?tab=readme-ov-file#ignore-a-module

patricklx commented 2 days ago

this is what i see in the deps of a vanilla vite repo: image

NullVoxPopuli commented 2 days ago

+1 to the browser field.

As is, loading content-tag in the browser is currently hinky requires patches/hacks). https://github.com/embroider-build/content-tag/blob/main/package.json#L13

patricklx commented 2 days ago

some parts are related to this code: https://github.com/embroider-build/embroider/blob/main/packages/vite/src/esbuild-request.ts#L199

it would also handle build-ins like path, but those are handled by vite with a custom namespace

ef4 commented 2 days ago

I created a minimal reproduction of this bug in esbuild: https://github.com/evanw/esbuild/issues/3976

The summary is, esbuild knows to consider certain paths "disabled" by the browser entry in package.json, but it hides this fact from plugins. There's no way to round-trip the PathDisabled flag through a plugin in a way that esbuild itself will respect.