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

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 this issue Nov 12, 2024 · 11 comments

Comments

@johanrd
Copy link
Contributor

johanrd commented Nov 12, 2024

Steps to reproduce

  1. Install the npm library sanitize-html, 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

  1. 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 #1583.

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

@patricklx
Copy link
Contributor

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
Copy link
Contributor Author

johanrd commented Nov 12, 2024

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/[email protected]/node_modules/postcss/lib/terminal-highlight

    node_modules/.pnpm/[email protected]/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/[email protected]/node_modules/postcss/lib/css-syntax-error.js:4:32: ERROR: Could not read from file: ~/ember-vite-npm/node_modules/.pnpm/[email protected]/node_modules/postcss/lib/terminal-highlight
    at failureErrorWithLog (~/ember-vite-npm/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1472:15)
    at ~/ember-vite-npm/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:945:25
    at ~/ember-vite-npm/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1353:9
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

@patricklx
Copy link
Contributor

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

@patricklx
Copy link
Contributor

patricklx commented Nov 12, 2024

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
Copy link
Member

mansona commented Nov 12, 2024

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
Copy link
Contributor

@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
Copy link
Contributor

patricklx commented Nov 12, 2024

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
Copy link
Contributor

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

@NullVoxPopuli
Copy link
Collaborator

+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
Copy link
Contributor

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
Copy link
Contributor

ef4 commented Nov 12, 2024

I created a minimal reproduction of this bug in esbuild: evanw/esbuild#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.

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

5 participants