Show console.warn only in production

[okletstryit]

make console.warn optional reading NODE_ENV variable (if exists). Show console.warn only in production.

The problem is that we log all console stuff to the logger and now we got thousands of useless messages there

[mroderick]

Why is your code loading PubSub more than once in node?

Can you give a usage example that reproduces the undesirable behaviour?

[adamstankiewicz]

@mroderick @okletstryit FWIW, we are also seeing the console.warn('PubSub already loaded, using existing version') message. We use PubSub as part of our @edx/frontend-platform micro-frontend application framework (source).

We don't see the console.warn when running consuming applications (e.g., in Webpack dev server); we do see it, however, when running tests for components in micro-frontend applications that consume the @edx/frontend-platform library.

Generally, the warning is logged roughly once per test file in consuming applications, which adds quite a lot of unnecessary/unwanted noise in our test output, and makes it appear as if there may be something wrong with the components we are testing even though they are passing tests, e.g:

 PASS  src/components/subscriptions/tests/licenses/LicenseAllocationHeader.test.jsx
  ● Console

    console.warn
      PubSub already loaded, using existing version

      at node_modules/pubsub-js/src/pubsub.js:15:17
      at Object.<anonymous> (node_modules/pubsub-js/src/pubsub.js:35:2)
      at Object.<anonymous> (node_modules/@edx/src/pubSub.js:20:1)

and

 PASS  src/components/CodeSearchResults/CodeSearchResults.test.jsx
  ● Console

    console.warn
      PubSub already loaded, using existing version

       8 | import isNumeric from 'validator/lib/isNumeric';
       9 |
    > 10 | import { history } from '@edx/frontend-platform/initialize';
         | ^
      11 | import { features } from './config';
      12 |
      13 | import {

      at node_modules/pubsub-js/src/pubsub.js:15:17
      at Object.<anonymous> (node_modules/pubsub-js/src/pubsub.js:35:2)
      at Object.<anonymous> (node_modules/@edx/src/pubSub.js:20:1)
      at Object.<anonymous> (node_modules/@edx/src/initialize.js:49:1)
      at Object.<anonymous> (src/utils.js:10:1)
      at Object.<anonymous> (src/components/CodeSearchResults/index.jsx:5:1)
      at Object.<anonymous> (src/components/CodeSearchResults/CodeSearchResults.test.jsx:9:1)

• I wonder, is there a different way we should be exposing/using PubSub in our @edx/frontend-platform library you might recommend?
• Since we're only seeing this warning in tests, we're also potentially interested in a solution to silence this warning if there's not a relatively simple workaround.

We've done some initial investigation here: openedx/wg-frontend#124

[arbrandes]

@mroderick, @okletstryit, friendly ping on Adam's comment above. We'd appreciate any help. Thanks!

[mroderick]

Thank you for the PR and the discussion.

I'm not sure that silencing the warning if running in non-production node environments is the appropriate solution. That will likely cause people to overlook this warning, which is meant to help them detect silly situations BEFORE they get to production.

I can imagine a few other solutions:

1. Use console.debug instead of console.warn ... if people are dumping console.debug into their logger, then they probably do that intentionally
2. Detect an environment variable that can disable the warning: PUBSUBJS_DISABLE_WARNINGS or something like that

Have you considered if solutions like that are appropriate for your scenario?

[adamstankiewicz]

@mroderick Thanks for the feedback and alternative solutions!

Use console.debug instead of console.warn ... if people are dumping console.debug into their logger, then they probably do that intentionally

I tried this by modifying the package's code in node_modules directly and the console.debug still comes through in the Jest test output, which is what we're trying to avoid, for example:

Detect an environment variable that can disable the warning: PUBSUBJS_DISABLE_WARNINGS or something like that

This would likely be a viable solution, where we could set such an environment variable in our setupTest.js file for Jest in our shared frontend config and have it apply to all our micro-frontends consuming that shared Jest config. Testing this approach locally seems to do the trick.

We'd be happy to open a PR to this repo if you'd also sold on the approach to only call console.warn when PUBSUBJS_DISABLE_EXISTING_VERSION_WARNING is falsey.

cc @arbrandes

[mroderick]

We'd be happy to open a PR to this repo if you'd also sold on the approach to only call console.warn when PUBSUBJS_DISABLE_EXISTING_VERSION_WARNING is falsey.

That would be much appreciated 👍

[adamstankiewicz]

@mroderick here is a PR to conditionally call the console.warn based on the environment variable:

#224

Feel free to review at your convenience. Thanks!