material-web icon indicating copy to clipboard operation
material-web copied to clipboard

@material/mwc-icon-button (and others) missing dependency, fails with yarn PnP

Open rhashimoto opened this issue 3 years ago • 3 comments

Describe the bug When using the yarn package manager with PnP, mwc-icon-button (at 0.25.3) is missing a dependency on @material/mwc-base, which is only provided as a devDependency.

To Reproduce Steps to reproduce the behavior:

  1. Create a yarn PnP project.
  2. yarn add @material/mwc-icon-button
  3. Create a source file that imports @material/mwc-icon-button.
  4. Bundle that file, e.g. using rollup and @rollup/plugin-node-resolve.

Expected behavior The source file should bundle without errors.

Observed behavior With rollup:

$ yarn rollup -c

foo.js → dist...
(!) Unresolved dependencies
https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency
@material/mwc-base/aria-property (imported by .yarn/cache/@material-mwc-icon-button-npm-0.25.3-96b7a8c03a-f756099514.zip/node_modules/@material/mwc-icon-button/mwc-icon-button-base.js)
(!) Missing global variable name
Use output.globals to specify browser global variable names corresponding to external modules
@material/mwc-base/aria-property (guessing 'ariaProperty')
created dist in 909ms

Additional context In the installed @material/mwc-icon-button package, the compiled file mwc-icon-button-base.js imports @material/mwc-base/aria-property but @material/mwc-base is only provided as a devDependency. I think this should be a regular dependency instead, or perhaps it was supposed to be incorporated into the compiled file.

This doesn't seem to produce an error with npm or yarn when configured to use node_modules, but breaks with PnP because it is more strict with enforcing dependencies.

A workaround with yarn PnP is to tell yarn to inject the missing dependency using the .yarnrc.yml file by adding something like:

packageExtensions:
  "@material/mwc-icon-button@^0.25.3":
    dependencies:
      "@material/mwc-base": "*"

There might be other similar problems in other packages; I haven't looked to see if this is a general problem.

rhashimoto avatar Jan 01 '22 19:01 rhashimoto

I noticed a similar issue with mwc-button as mwc-base is not included as a dependency.

jeffgsk avatar Jan 04 '22 17:01 jeffgsk

I just created a PR to fix the same issue, should have checked first :D Thanks @rhashimoto

Should we maybe create a PR to fix all other ambiguous dependencies? Would it be welcome by the Google team?

espoal avatar Jan 07 '22 13:01 espoal

I just created a PR to fix the same issue, should have checked first :D Thanks @rhashimoto

Should we maybe create a PR to fix all other ambiguous dependencies? Would it be welcome by the Google team?

It would be welcome! Sorry for the delay on this, we're heads down focused on Material You 😅

asyncliz avatar Mar 23 '22 18:03 asyncliz