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

Many "Inferno is in development mode" logs during testing #1573

Closed
jrwdexter opened this issue Aug 15, 2021 · 11 comments · Fixed by #1581
Closed

Many "Inferno is in development mode" logs during testing #1573

jrwdexter opened this issue Aug 15, 2021 · 11 comments · Fixed by #1581

Comments

@jrwdexter
Copy link

Observed Behaviour

When running tests that utilize Inferno as part of the framework, every test will log Inferno is in development mode. to the console. This is due to the NODE_ENV !== 'production' check in index.ts within Inferno.

This logging behavior, without an ability to stop it, pollutes test logs with these statements.

Expected Current Behaviour

I don't know if there's an exact specified behavior. Having a way to turn off these logs (without setting the whole process to 'production') would be helpful.

If this is a design choice, the options as far as I can tell are to:

  • Add a new environment variable (or other setting) to disable Inferno log statements
  • Detect a variable when this log should happen - to ensure it gets run when a "real" browser is running, and not a test suite.
  • "Just deal with" extra log statements in various capacities
  • Ask users to monkey patch console.log to avoid extra logging
  • Ask users to switch test suites running to 'production'

A few of the above are non-ideal, and I would argue switching test suite to production is just a bad idea.

Ideally, I think that an environment variable or environment detection makes the most sense.

@Havunen
Copy link
Member

Havunen commented Aug 16, 2021

Hi

I think you have something wrong in your test suite build if you see more than one message it means you have more than one infernojs bundled in your javascript bundle. Is that intentional?

@igvnv
Copy link
Contributor

igvnv commented Sep 8, 2021

I have this console.log for each testing suite. But, to be honest, I don't want console.log at all, even in browser.

@Havunen
Copy link
Member

Havunen commented Sep 8, 2021

what about disabling the console log then?

window.console.log = () => {}

@igvnv
Copy link
Contributor

igvnv commented Sep 8, 2021

Monkey patching? I once did this and then lost some valuable data in tests. It was my bad, should never rely on console.log, but some 3rd-party libraries still can use it.

Is there a need for displaying this message? I understand and appreciate that the library displays console.warn when I use minified version during developing. But if every library will spam in console with "you are in development mode", console will become a useless place.

@Havunen
Copy link
Member

Havunen commented Sep 8, 2021

It is valuable information because people do bundling mistakes and then report issues to inferno saying its slow or post benchmarks to internet and don't even use optimized version, well yeah its slow if the build is not even in production mode.

@igvnv
Copy link
Contributor

igvnv commented Sep 8, 2021

Understandable.

I suppose that wrapping this console.log into if (process.env.NODE_ENV !== 'test') won't affect developers and production users but will add a small improvement for developing process. It is not a big change, but even small changes make us happier.

@Havunen
Copy link
Member

Havunen commented Sep 8, 2021

Okay sure, would you like to send PR?

alfeg added a commit to alfeg/inferno that referenced this issue Nov 6, 2021
Simple fix suggested by @igvnv infernojs#1573 (comment)

This should cleanup console output during jest test run.

Jest runner ensure that `process.env.NODE_ENV` is set to `test`
@alfeg alfeg mentioned this issue Nov 6, 2021
@igvnv igvnv mentioned this issue Nov 8, 2021
Havunen pushed a commit that referenced this issue Nov 8, 2021
* skipping developement mode notification in testing environment

* better environment type detection

Co-authored-by: Roman Petrov <roman.petrov@lucy-ims.com>
@p1100i
Copy link
Contributor

p1100i commented Feb 1, 2024

hi @Havunen, sorry to necrobump this, I ran into the same issue, in my case however i'm not using jest, but another test framework, also i'm executing specs in the browser (without process object).

I was wondering if you would be open for a PR which "generalizes" this warning-skipping logic to cover both node and browser environment and renaming into something like SKIP_INFERNO_WARNINGS?

@Havunen
Copy link
Member

Havunen commented Feb 1, 2024

I was wondering if you would be open for a PR which "generalizes" this warning-skipping logic to cover both node and browser environment and renaming into something like SKIP_INFERNO_WARNINGS?

Sounds good to me as long as the code can be treeshaken off from the production bundle

Havunen pushed a commit that referenced this issue Feb 4, 2024
* adds checking of variable `SKIP_INFERNO_WARNINGS` to skip warnings
  in non-production mode

* `SKIP_INFERNO_WARNINGS` can be also declared globally for browser
  functionality, or just defined on the process.env in case of node

* FYI: JEST related warning skip about development mode was removed
  with 2ed75dc when fixing #1582, only the skipping of
  minified code warning was in place
@iambumblehead
Copy link
Contributor

Probably there are good reasons for the message but here they seem to be the wrong solution for whatever problem is resolved. Why would a package ship with "development" mode as default rather than opt-in behaviour? Runtime configurable settings would be more user-friendly, eg render(vtree, rootvdom, { IS_DEVELOPMENT_MODE: true })

@Havunen
Copy link
Member

Havunen commented Aug 1, 2024

Probably there are good reasons for the message but here they seem to be the wrong solution for whatever problem is resolved. Why would a package ship with "development" mode as default rather than opt-in behaviour? Runtime configurable settings would be more user-friendly, eg render(vtree, rootvdom, { IS_DEVELOPMENT_MODE: true })

The reasoning is following: if you start developing new application and you are new to infernojs. If it would have production mode enabled, then you will not see any of the warnings produced by the development time validations and you may encounter very weird behavior. For example using duplicate "key" wouldnt produce error, or missing "key" could go into production unnoticed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants