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

How to deal with webpack warning from unmaintained grandparent module? #1924

Closed
davidascher opened this issue Apr 2, 2017 · 8 comments
Closed

Comments

@davidascher
Copy link
Contributor

davidascher commented Apr 2, 2017

Can you reproduce the problem with latest npm?

yes.

Description

Requiring winston generates a warning due to some webpack-warning-deserving code that winston's dependency, colors has.

It's currently impossible AFAICT to tell eslint to ignore it.

I'm opening this bug here because it's impossible to e.g. disable the webpack warning exprContextCritical without ejecting, but using a very popular logging module shouldn't require ejecting.

Alternatively, can anyone suggest an alternative to winston?

Related issues:

Expected behavior

Adding winston to a CRA project should not add warnings on each page load, or it should be possible to deal with a "misbehaving" grandparent.

Actual behavior

import log from "winston";
log.info('hi there')

yields:

Compiled with warnings.

Warning in ./~/colors/lib/colors.js
Critical dependencies:
127:29-43 the request of a dependency is an expression
 @ ./~/colors/lib/colors.js 127:29-43

Warning in ./~/colors/lib/extendStringPrototype.js
Critical dependencies:
106:31-45 the request of a dependency is an expression
 @ ./~/colors/lib/extendStringPrototype.js 106:31-45

You may use special comments to disable some warnings.
Use // eslint-disable-next-line to ignore the next line.
Use /* eslint-disable */ to ignore all warnings in a file.

Environment

  1. npm ls react-scripts (if you haven’t ejected):
[email protected] /Users/da/src/webpack-warning
└── [email protected] 
  1. node -v: v7.7.2
  2. npm -v: 4.4.4

Then, specify:

  1. Operating system: OSX 10.12.4
  2. Browser and version: Chrome 56.0.2924.87

Reproducible Demo

See minimal repo at https://github.com/davidascher/webpack-warning/

@davidascher davidascher changed the title How to deal with unmaintained grandparent webpack warning? How to deal with webpack warning from unmaintained grandparent module? Apr 2, 2017
@davidascher davidascher changed the title How to deal with webpack warning from unmaintained grandparent module? How to deal with eslint warning from unmaintained grandparent module? Apr 2, 2017
@davidascher davidascher changed the title How to deal with eslint warning from unmaintained grandparent module? How to deal with webpack warning from unmaintained grandparent module? Apr 2, 2017
@Timer
Copy link
Contributor

Timer commented Apr 2, 2017

I suggest forking colors, removing the dynamic require (or putting it behind a nodejs/webpack&browserify compatible escape hatch to make it web bundle friendly), and releasing a semver-major (2.0).
You can then submit a follow up PR to winston switching to the new module.

Unfortunately we're not going to add an escape hatch for this, so you will need to eject or live with the warning.
Our philosophy is to require no configuration and avoid introducing it where otherwise unnecessary. Adding an escape hatch to bundle improper web packages is one we're definitely not going to support (sorry!).


You can check out bunyan, too.

Let me know if we can help with anything else. :)

@gaearon
Copy link
Contributor

gaearon commented Apr 2, 2017

Longer term we aim to solve it here: webpack/webpack#4263

@timse
Copy link

timse commented Apr 3, 2017

Not sure how you plan to fix it, but webpack/webpack#4381 was merged, once released you can use it to suppress those warnings.
Would be happy to do a PR but not sure how you plan and what you plan to ignore :)

@Timer
Copy link
Contributor

Timer commented Apr 3, 2017

Oo that looks promising @timse. Can you please open a new proposal issue for us to talk details? Thanks!

@shai32
Copy link

shai32 commented Apr 3, 2017

Longer term we aim to solve it here: webpack/webpack#4263

@gaearon now that the suppress warnings feature was added to webpack, do you plan to use it ?
maybe open this issue again?

@timse
Copy link

timse commented Apr 7, 2017

ill reopen an issue :)

@shai32
Copy link

shai32 commented Apr 12, 2017

Can you add a link to the new issue?

@Timer
Copy link
Contributor

Timer commented Apr 12, 2017

It's linked above your comment, #1947.

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants