saadeghi / daisyui

🌼 🌼 🌼 🌼 🌼  The most popular, free and open-source Tailwind CSS component library
https://daisyui.com
MIT License
34.14k stars 1.3k forks source link

bug: missing transitive `postcss` peer dependency #3191

Closed rtritto closed 2 months ago

rtritto commented 2 months ago

What version of daisyUI are you using?

v4.12.10

Which browsers are you seeing the problem on?

No response

Reproduction URL

NA

Describe your issue

postcss-js has postcss as peer dependency. daisyui use postcss-js dependency and it should also include postcss as dependency or peer dependency.

yarn explain peer-requirements
...
p95141 β†’ ✘ daisyui@npm:4.12.10 doesn't provide postcss to postcss-js@npm:4.0.1 [983eb]
...
github-actions[bot] commented 2 months ago

Thank you @rtritto for reporting issues. It helps daisyUI a lot πŸ’š
I'll be working on issues one by one. I will help with this one as soon as a I find a solution.
In the meantime providing more details and reproduction links would be helpful.

saadeghi commented 2 months ago

Can you please describe the issue?

Offering a solution needs describing the problem first.

rtritto commented 2 months ago

daisyui use postcss-js dependency with no postcss dependency (postcss is a peer dependency of postcss-js). If you use a dependency, you must also add all peer dependencies of that dependency (transitive dependencies).

saadeghi commented 2 months ago

My friend, are you having an issue on installation in any environment?

rtritto commented 2 months ago

I'm using yarn berry and with yarn explain peer-requirements, I got the warning as missing dependency (not-compliant).

peer-dependency

I have to use a workaround for that issue (missing dependency):

.yarnrc.yml

+packageExtensions:
+  daisyui@*:
+    dependencies:
+      postcss: '*'
rtritto commented 2 months ago

Probably you are using a package manager that autoinstall transitive peer dependencies (shadow behavior) and the issue is hidden.

saadeghi commented 2 months ago

Thanks for more explanation.
Is this warning preventing daisyUI to work?

Because what you're suggesting is not logical. A dependency can not possibly list all the peer dependencies of its dependencies as peer dependency. Each dependency is a independent project. They can add or remove their dependencies any time. Does it make sense that a dependant package should monitor and list all peer dependencies of all dependencies and sub dependencies and...??

rtritto commented 2 months ago

Is this warning preventing daisyUI to work?

It's a warning, so no, it's not blocking. I can use the workaroud until the fix.

Because what you're suggesting is not logical. A dependency can not possibly list all the peer dependencies of its dependencies as peer dependency. Each dependency is a independent project. They can add or remove their dependencies any time. Does it make sense that a dependant package should monitor and list all peer dependencies of all dependencies and sub dependencies and...??

Peer dependencies aren't installed by all package managers. That's why you see a peer dependency also in the dev dependencies (the peer dependency is used in tests or in build).

All plugin libraries have that logic. E.g.:

All projects work using that logic as standard.

From answer https://stackoverflow.com/a/68575682 and source https://yarnpkg.com/advanced/error-codes#yn0002---missing_peer_dependency

# bad
project
β”œβ”€D> packagePeer
└─D> packageA
     └─P> packageB
          └─P> packagePeer

# good
project
β”œβ”€D> packagePeer
└─D> packageA
     β”œβ”€P> packagePeer
     └─D> packageB
          └─P> packagePeer
rtritto commented 2 months ago

@saadeghi if it's not enough, I can tag other expert users to exaplain better this issue Thanks for your support

saadeghi commented 2 months ago

Post CSS is not required for daisyUI to work.

postcss-js does not require postcss to work, neither does daisyui

So I think you should either ignore the warning, or disable it.

Warning β‰  Error. Warnings exist to warn you about a potential problem, but if you know it's not a problem, you shouldn't pay attention. It's a console log after all. It's not that smart πŸ˜…

rtritto commented 2 months ago

postcss-js does not require postcss to work, neither does daisyui

package.json of postcss-js include in peer dependencies:

https://github.com/postcss/postcss-js/blob/b3db658b932b42f6ac14ca0b1d50f50c4569805b/package.json#L48-L50

  "peerDependencies": {
    "postcss": "^8.4.21"
  },

To have postcss as optional peer dependency, we need this:

  "peerDependenciesMeta": {
    "postcss": {
      "optional": true
    }
  }
rtritto commented 2 months ago

Warning β‰  Error. Warnings exist to warn you about a potential problem, but if you know it's not a problem, you shouldn't pay attention. It's a console log after all. It's not that smart πŸ˜…

That's because yarn berry autoinstall missing peer dependencies to not break dependency tree resolution. The warning says that something isn't how should be.

saadeghi commented 2 months ago

Even if it doesn't install postcss , nothing would break. daisyUI doesn't depend on postcss.
If it was depending, I would've add it to dependencies. Like other dependencies.

rtritto commented 2 months ago

Even if it doesn't install postcss , nothing would break. daisyUI doesn't depend on postcss. If it was depending, I would've add it to dependencies. Like other dependencies.

postcss-js depends on postcss (package.json of postcss-js have postcss in peer dependencies). In a project, if you want use postcss-js dependency, you need also include postcss as dependency.

So, if DaisyUI uses postcss-js dependency, postcss must be added in dependencies.

rtritto commented 2 months ago

You can check in node_modules directory and you will find postcss directory because it it's autoinstalled by default (shadow install) by package manager (NPM).

DaisyUI should explicit postcss dependency because is used by postcss-js the because package manager must install also postcss dependency.

rtritto commented 2 months ago

Is this warning preventing daisyUI to work?

It's a warning, so no, it's not blocking. I can use the workaroud until the fix.

Because what you're suggesting is not logical. A dependency can not possibly list all the peer dependencies of its dependencies as peer dependency. Each dependency is a independent project. They can add or remove their dependencies any time. Does it make sense that a dependant package should monitor and list all peer dependencies of all dependencies and sub dependencies and...??

Peer dependencies aren't installed by all package managers. That's why you see a peer dependency also in the dev dependencies (the peer dependency is used in tests or in build).

All plugin libraries have that logic. E.g.:

* in a project, you add the dependency A and you want add the plugin B for the dependency A

* plugin B have dependency A in peer dependencies (so the issue will not appear)

* plugin B can't work without dependency A (when you add B you must also add A)

All projects work using that logic as standard.

From answer https://stackoverflow.com/a/68575682 and source https://yarnpkg.com/advanced/error-codes#yn0002---missing_peer_dependency

# bad
project
β”œβ”€D> packagePeer
└─D> packageA
     └─P> packageB
          └─P> packagePeer

# good
project
β”œβ”€D> packagePeer
└─D> packageA
     β”œβ”€P> packagePeer
     └─D> packageB
          └─P> packagePeer

All this is well documented on yarn site.

saadeghi commented 2 months ago

In a project, if you want use postcss-js dependency, you need also include postcss as dependency.

That's not true. No one adds all the peer dependencies of all their dependencies (and sub dependencies) as peer dependency. It's not realistic and no package does it.

There are packages out there with 100+ dependencies. Each dependency has multiple other dependencies. All of them have their own peer dependencies. With every release any package can add or remove any peer dependency. The consumer package can not possibly keep track of all those peer dependencies and manually(?) add/remove them to their package.json file. It's even funny to do such thing.

I appreciate your effort for the PR and opening the issue. And thanks for the conversation. Unfortunately it's not something we can add unless we start using the postcss package directly.

With the upcoming version of Tailwind (4) and the upcoming version of daisyUI (5), there will be even less dependencies hopefully and we will even remove postcss-js.

rtritto commented 2 months ago

That's not true. No one adds all the peer dependencies of all their dependencies (and sub dependencies) as peer dependency. It's not realistic and no package does it.

You can check, almost all libraries add a dependency (because it's listed in the peer dependencies) and doesn't use it directly in the code.

There are packages out there with 100+ dependencies. Each dependency has multiple other dependencies. All of them have their own peer dependencies. With every release any package can add or remove any peer dependency. The consumer package can not possibly keep track of all those peer dependencies and manually(?) add/remove them to their package.json file. It's even funny to do such thing.

Peer dependency mean that all dependencies in that list are not installed but the project still use them. When you add a dependency that have peer dependency, you install only dependencies. That dependency is partial so you need the add all peer dependencies as dependency to have the dependency tree completed (lock file). If you do a clear install (delete node_modules and then install), on logs, you will see the warning of a missing peer dependency as dependency.

When a peer dependency is removed, a major version of that library is released. This helps when migrating the version.

With the upcoming version of Tailwind (4) and the upcoming version of daisyUI (5), there will be even less dependencies hopefully and we will even remove postcss-js.

The issue isn't fixed, I will keep this issue opened to advise and help (with workaround or other) users.

rtritto commented 2 months ago

An article written by author of yarn package manager: Implicit Transitive Peer Dependencies

Source and some info in comments: https://github.com/preactjs/next-plugin-preact/issues/46#issuecomment-913872884

FYI @saadeghi

robinelvin commented 3 days ago

I've just come up against this in my Yarn PnP-managed project. I've tried manually specifying the dependency using .yarnrc.yml but it seems I can't do that. My project does not build so this is not just a warning that I can ignore.

The error:

Unhandled promise rejection: Error: [postcss] postcss-js tried to access postcss (a peer dependency) but it isn't provided by its ancestors; this makes the require call ambiguous and unsound.

Required package: postcss
Required by: postcss-js@virtual:6a3cfa3c2d0544590f360eb7e9412b13b21b3c1430fca94fa0aea7f53b52a2bcc623d94a736c469426bd7377028e0391e039300a875bfaf91b426d26d2851197#npm:4.0.1 (via /Volumes/Crucial X8/Projects/develop/.yarn/__virtual__/postcss-js-virtual-b2253b7e37/0/cache/postcss-js-npm-4.0.1-2c4ee70bf3-ef2cfe8554.zip/node_modules/postcss-js/)

Ancestor breaking the chain: daisyui@npm:4.12.14
$ yarn explain peer-requirements

... (excerpt for brevity)
p01f2b β†’ βœ“ cross-fetch@npm:4.0.0 doesn't provide @types/encoding to node-fetch@npm:2.7.0 [1c2a4]
p9a3aa β†’ βœ“ cross-fetch@npm:4.0.0 doesn't provide encoding to node-fetch@npm:2.7.0 [1c2a4]
pe9ec3 β†’ βœ“ daisyui@npm:4.12.14 doesn't provide @types/postcss to postcss-js@npm:4.0.1 [6a3cf]
pae8ba β†’ ✘ daisyui@npm:4.12.14 doesn't provide postcss to postcss-js@npm:4.0.1 [6a3cf]
p83015 β†’ βœ“ docker-modem@npm:3.0.8 doesn't provide @types/supports-color to debug@npm:4.3.4 [3837a]
p5186f β†’ βœ“ docker-modem@npm:3.0.8 doesn't provide supports-color to debug@npm:4.3.4 [3837a]
...
rtritto commented 3 days ago

For now, this bug is a won't fix.

We can wait the Tailwind v4 support and hope that there isn't the same problem with other dependencies.

Meanwhile you can use this workaround to install the missing postcss transitive dependency: https://github.com/saadeghi/daisyui/issues/3191#issuecomment-2322862312

FYI @saadeghi