make logging optional for webgl renderer#5835
Conversation
|
Why would one not want logging on production? |
|
We could combine into this: #5791 -- Just add another THREE.log(). It would feed all through the same system. |
|
@mrdoob the production case for this change might not be as strong as the test one (which would be that there is test pollution any time a renderer is initialized) -- but I believe the strongest case for prod is that no other JS framework that I've used/seen automatically logs out purely informational content like this without allowing you to turn it off? Regardless, the test situation is much more relevant for test runners that will log out things like this. Aside: loving this library and the work you've done 🤘 |
|
@bhouston I definitely think that is the right direction! If you implemented that Ideally, one could set a global |
|
@JackCA I've made the change on my PR (and simplified the log handlers too.) You can yourself add listeners to THREE.warning, THREE.error and THREE.log to add a logLevel-like handler. Would that satisfy your needs? I'm concerned about adding too much logging infrastructure to THREE.js as @mrdoob makes a lot of effort to minimize the code ThreeJS size. BTW if you support my PR, please test it and inform me on the PR if it works for your needs. The more in support of the PR, the more likely it is to get merged. :) |
|
Personally, I would prefer that this single line remain required. |
|
Yeah, me too... That line is very useful when helping people. If we allow them to hide that message and they have some iframe or require.js business going there is no way to know what version they're using. |
|
@mrdoob I'm not sure what situation someone would be in where they consciously disable logging, yet don't know what version of three.js they're using... I've never seen mandatory logging like this before and I think there is a lot of precedence and good reasons for making it optional, but if you're against that, then feel free to close this PR. |
|
Oh. You'll be surprised. Humans are very unreliable.
Do you know any articles on the subject? |
This reverts commit 632002c. Conflicts: src/Three.js
|
The king of logging frameworks is log4j (4j = For Java) which has evolved into the Apache Logging System: http://logging.apache.org/ Architecture here: http://logging.apache.org/log4j/2.x/manual/architecture.html The JavaScript / NPM version is here: https://github.com/nomiddlename/log4js-node |
|
@mrdoob I don't have any articles off hand -- but just look at angular, ember, jQuery, etc. None of them have a mandatory log out to the console anywhere. Why would Three.js be any different? Those libraries have more users, more use-cases, more versions, and more that can go wrong, yet they get away with it. I'm a fan of how ember does it: http://emberjs.com/guides/understanding-ember/debugging -- they're using something similar to what @bhouston did. Also see generic solutions such as: https://github.com/pimterry/loglevel. I'm sure you've run into a lot of situations where people don't know what version they're using and don't know how to run But I'm not even suggesting you remove it -- I've retained the current functionality where the default setting is to still log out the version number. I think @bhouston is on the right track with the logging wrappers, but with your recent revert on the change that was relevant to this PR, I can't suggest it for my purposes -- it should be all or nothing. My use case is purely for integration testing: I'm using dozens and dozens of libraries and the only thing polluting my test runners output is this one line. Think it's an anti-pattern to be noisy regardless of the desire of the developer which is why you don't see |
It is also useful in seeing where three.js is being used on the net. |
|
@WestLangley you can find that out via the existence of |
In addition to what @JackCA said, both of the above require the user to open the development console and are therefore only useful to developers. Such users probably know how to type If you want to see where three.js is used (and get some publicity), wouldn't it be better to create a nice, official icon and suggest people put it on their website? That's what I've seen with other libraries like those @JackCA mentioned in his previous post. |
This is not about publicity... |
How would you do that in this jsfiddle, for example? |
|
Notice the context of the debugger is changed to the result of the jsfiddle |
|
@JackCA: @mrdoob and @WestLangley are right that in complex applications, it can be hard to find the THREE variable. I know if you are using a module-based system (such as RequireJS or similar, e.g. https://www.npmjs.com/package/three), THREE won't be a first-level global variable but may be arbitrarily nested somewhere. |
|
@JackCA Thank you for the jsfiddle solution. : - ) |
|
@bhouston If someone is using a relatively complex system like RequireJS and they don't know how to figure out what version of three.js they are including, I think they've got bigger problems... Regardless: I'm trying to see if there is a compelling reason for why it should be mandatory to log this out. Everyone has chimed in on why it's nice to have/helpful in the first place, but not why we shouldn't be able to silence it if desired. |
|
@JackCA We have a complex unit and integration test framework and we've never had an issue with unwanted console.log output -- we check for specific things in our tests and ignore console.log completely. This generally works for us. Thus while it may be a bit cleaner to be able to redirect ThreeJS's console.log output, I can not see why it MUST be redirected, it seems like an aesthetic choice either way. Generally I am not familiar with testing frameworks that look at console.log and depend on it to be perfectly clean. In fact I am usually used to adding console.log/console.trace statements and other messy things while debugging failing unit tests |
|
@bhouston my testing framework/runner doesn't depend on a clean output -- it's a matter of having a clean visual output at the end of the test. If something goes wrong, then I expect to start seeing some logging and traces. It's definitely a matter of aesthetics but I know that it is a common practice to not expect application logic to cause logging during tests (especially during passing tests). I think the term "test pollution" refers directly to this type of thing. |
I think a good reason why this should not be removed is the fact that three.js should get credit for their library. Three doesn't require us to put a "Powered by Three.js" or some link on our projects, the least we could do is leave it in the console. If it is too bothersome, why not simply comment out the line in src? |
This is absolutely an awesome library and definitely deserves some credit, but quoting @mrdoob earlier:
And your other question:
I use bower so I wouldn't be able to do that cleanly. |
|
I just want to add that I've never seen a production JavaScript library produce any output by default (without debug enabled). That said, it's obviously not a big deal. I really don't care either way! 📟 |
Both. You ask your users to provide enough debugging information when asking for help, just like good engineers should be doing. You can only help those that want to help themselves. |
|
looking at console isn't the only way of determining version, is it? why not check in package.json? or just export a global variable called THREE_VERSION or something like that and let them give you version that way. Or I could argue with having side effects, logging would be a side effect, and should be avoided, just log version when I ask it to log, say THREE.printVersion() or something like that, but it's doing something user hasn't explicitly asked to do. Extension approach sounds better for me. |
|
I've been working a long long time with WebGL, but mostly vanilla or with Regl. Now I had the chance to start some client work with Three.js. The client is quite tech savvy and checked the console for his new website. The console message was unacceptable to them, so I had to monkey patch the console object to prevent the message from appearing (which makes me feel dirty 😿). I understand the argument for maintainers sake, but from a business perspective this PR should have landed a long time ago. Still, big thank you to all the maintainers, I've never been able to get out impressive WebGL stuff so quickly! |
|
actually not only version, but webgl could have a logger, and it could default to verbose, but as developers, if we could set the log level to error, and hide all the info and warnings in production, that'd be great. |
|
This discussion seems needlessly stubborn, can we PLEASE just have that one line line be optional? Make it enabled by default if you must, but PLEASE make it optional. 🙏 |
|
Anyway here's a little helper for anyone else who's annoyed by this. const filterLogging = (undesiredPattern, code) => {
const originalLog = console.log;
console.log = function (...args) {
return originalLog(...args && args.join(' ').match(undesiredPattern) ? args : []);
};
code();
console.log = originalLog;
};Usage: filterLogging(/^THREE\.WebGLRenderer\s*\d*$/, () => {
this.renderer = new THREE.WebGLRenderer();
}); |
|
I modify the build itself - though I know this isn't good practice and I need to do it again once I update to another version - by wrapping up the code like this (function(console) {console.log = function() {};
// Code from build/three.js
})(Object.create(console));I can imagine that using build systems like webpack this line could be added programmatically. |
|
Since this is a closed pull request and won't be reopened, where is the issue that is handling this problem? I've read most of this discussion and there are a lot of reasons for making it optional, and there are a lot of good ideas for fixing it, but nothing seems to be happening with it. It was suggested to open a new issue, but any attempts for new issues or new PRs just get marked as duplicates of this PR (here, here, here, here, here), which is closed. So what are the next steps? I'd like to get the single console log of the version removed in my own website (none of my other libraries have any required logs). IMO, I feel that any website that uses ThreeJS (or any library) should have control over the console logging, and not the library itself, since the developers of the website know best what their clients want. In this particular case, perhaps the website should simply be able to query ThreeJS for the version and console log if it wants. Something as simple as |
|
@stephenhurleyjpl wrap |
|
@dmarcos I'll do that as a temporary solution, but that is just a band-aid hack (modifying console.log) until a proper solution is found. Just imagine if modifying the built-in console.log was required by everyone's library for removing all output in production code. Still waiting on a response for how to move this issue forward. |
|
if you use webpack you can use the terser plugin |
|
@dmarcos Sorry, I commented to you on my other account accidentally, but I'm the same person as @fiddleplum (work vs home). |
|
Console.log removed. 1013fbb |
|
unsubscribe. |
|
Finally, some closure! 🙏 |
|
Argh, all of my tests are now failing because of this breaking change! |
|
It seems no matter what we do with this issue, someone will always complain 😞 |
|
@jonny-improbable You're over-testing, then. That is not Three.js's problem. I suggest researching how to do proper testing. Google is a great resource. This issue should probably be locked as "Resolved". |
|
How would this be a breaking change? You probably should test better. |
|
@jonny-improbable can you elaborate on how removing a |
|
Wait... @jonny-improbable, are you suggesting that asserting a third-party library calling |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Has setting a global variable been considered to allow curious devs to inspect the version number of other projects? Also I'm interested to know why the implementation hasn't been made optional like the original PR? |
|
Because browser APIs aren't designed with "curious devs" in mind, but instead the actual devs. If everything was reverse engineer friendly, there'd be a lot of extra cruft in the world. |
I wish it had been. |
|
@funwithtriangles we're moving to devtools: https://github.com/threejs/three-devtools But, we forgot to expose the revision in r108, should be fixed in r109: #17342 |

Adds a
loggingparameter to the WebGL renderer.Useful for situations where you don't want logging at all such as production or in a testing environment. Retains
trueas default but would thinkfalsewould be a better option?