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

soundmanger2.js console.log #13

Closed
patrickpoh opened this issue Aug 16, 2016 · 22 comments
Closed

soundmanger2.js console.log #13

patrickpoh opened this issue Aug 16, 2016 · 22 comments

Comments

@patrickpoh
Copy link

Hi! Excellent library that you have made! However, the amount of stuff getting console.log is humongous!
Is there anyway to remove it? It's coming from soundmanger2.js.

@patrickpoh
Copy link
Author

patrickpoh commented Aug 16, 2016

Hi! I editted the index.js in the react-sound folder where i change var _soundmanger2 = require('soundmanger2') to var _soundmanager2 = require('soundmanager2/script/soundmanager2-nodebug-jsmin.js');.

Is it possible for you to update it to that? so that all the debug messages wont appear in the console as it causes the app to be very laggy.

@leoasis
Copy link
Owner

leoasis commented Aug 17, 2016

Hi! Thanks! Glad that you're using the library!

Regarding the issue, check this thread: #8

And also this PR (still open) #12

I think maybe a good solution would be to include one or the other depending on the value of process.env.NODE_ENV?

@patrickpoh
Copy link
Author

hey! I used alias to resolve it! alias: { soundmanager2: 'soundmanager2/script/soundmanager2-nodebug-jsmin.js' },
Just included this in the webpack.config.js! hope this helps anyone who want to get rid of the console.log.

@juandjara
Copy link

You can also use this on your component with sound to disable the soundManager console messages.

componentDidMount () {
  soundManager.setup({debugMode: false});
}

That way you don't have to mess with the Webpack config (good if you are using something like create-react-app)

@jasan-s
Copy link

jasan-s commented Nov 29, 2016

@patrickpoh where in the web pack config file do I add that code?

@patrickpoh
Copy link
Author

@jasan-s check this link out --> https://github.com/andrewjmead/react-course-boilerplate-app/blob/master/webpack.config.js

@jasan-s
Copy link

jasan-s commented Nov 29, 2016

thanks @patrickpoh . that worked like a charm. Could changes in sound manager break my app because of this line of code in my webpack config?

@patrickpoh
Copy link
Author

@jasan-s you're welcome. Nope it wouldn't.

@leoasis
Copy link
Owner

leoasis commented Nov 29, 2016

Yes, the approach that @patrickpoh is mentioning is the one I'm using, and that works perfectly without any issues. However, maybe it would be good to try to automatically do that based on the NODE_ENV variable? That way the debug info won't ship to the prod bundle, and also this pattern is widely used in different libraries that are used with a bundler, be it webpack or browserify or whatever.

What do you people think? What should be the default behavior? Or just documenting this webpack config alias would be enough?

@jasan-s
Copy link

jasan-s commented Nov 29, 2016 via email

@rshkv
Copy link

rshkv commented Mar 12, 2017

+1 for not logging as default

@baharev
Copy link

baharev commented Aug 26, 2017

@juandjara I am precisely in the situation you write: I am using create-react-app, and I have no idea how to mess with the Webpack config because I am an absolute beginner.

How do you get hold of the soundManager?

I have tried:

import Sound, { soundManager } from 'react-sound';

but I get TypeError: __WEBPACK_IMPORTED_MODULE_2_react_sound__.soundManager is undefined on soundManager.setup({debugMode: false}); which is not surprising after looking at the source code of react-sound: It does not export the soundManager. Could you help me, please? How can I use the code you suggested?

@juandjara
Copy link

@baharev I write this code some time ago for my website http://freetunes.fuken.xyz so I do remember very well how I did it. But here is the code, https://github.com/juandjara/ftunes-client/blob/master/src/player/Player.js

It seems that you don't have to import sound-manager because it is already imported as a global when you import react-sound

@baharev
Copy link

baharev commented Aug 26, 2017

@juandjara Thanks for the quick response. In the index.js of my app, where I create the store, and have

ReactDOM.render(
  <Provider store={store}>
    <App />
  </Provider>,
  document.getElementById('root')
)

I inserted the following 3 lines after the imports but before anything else:

/* global soundManager:false */
import 'react-sound';
soundManager.setup({debugMode: false});

Seems to work OK.

Apparently the /* global soundManager:false */ does the trick. What is it called? And where I can read more about it?

And is the top level index.js of my app a safe place for these 3 lines?

I appreciate your help.

@juandjara
Copy link

juandjara commented Aug 27, 2017

@baharev well you can do it this way but I suggest you put it in the file of the component where you are actually using the <Sound /> component.

And the /* global soundManager:false */ has nothing to do with that. It will work without this comment. It's just a comment for eslint , a tool that checks if your code follows a certain style guide defined by eslint rules.

The thing that is happening here is this: When you import react-sound you run the code that sets soundManager as a variable in the global scope. You can check this by typing window.soundManager in your browser console.

@baharev
Copy link

baharev commented Aug 27, 2017

@juandjara Thanks for the explanation.

I suggest you put it in the file of the component where you are actually using the <Sound /> component.

Okay, and why?

Why is my workaround dangerous / suboptimal / wrong? When could it backfire?

What if I have 16 different components in my application that use <Sound />, and I cannot possibly know which one will be rendered first (because it depends on the user's decision)? Do I copy-and-paste that

componentDidMount () {
  soundManager.setup({debugMode: false});
}

code into all 16 components just in case that component is mounted first? And if 12 of them are pure functional components, do I have to turn all 12 of them into classes just to implement componentDidMount()?

Sorry if this is a dumb question, but I am an absolute beginner, and I would like to understand why the way that you are suggesting is better / safer than my workaround. Thanks for your patience.

@juandjara
Copy link

@baharev if that it's the case your workaround is totally fine. I wrote it that way because I only use react-sound in one component, for clarity, but you could write it on your index.js if you are using it that way.

@baharev
Copy link

baharev commented Aug 27, 2017

@juandjara That soundManager thingy looks like a global singleton to me. It makes more sense to me to set it exactly once, before the application starts, and in the file, where the "root things" are anyway (rootReduce, root DOM node is looked up, etc). And then I do not have to worry about any component in the application that later forgets to set it. OK, as far as I am concerned, this issue is resolved, thanks for the help!

@leoasis
Copy link
Owner

leoasis commented Dec 26, 2017

@patrykkowalski0617
Copy link

I use
import Sound from "react-sound-dkadrios";
instead
import Sound from "react-sound";
and it works fine!

@SaadRegal
Copy link

SaadRegal commented Sep 5, 2019

This one worked for me like a charm :

import  Sound from 'react-sound';

Then, add this to your constructor :

  constructor(props){
    window.soundManager.setup({debugMode: false});
    super(props);
}

Or simply use it this way

import  Sound from 'react-sound';
window.soundManager.setup({debugMode: false});

@tylim88
Copy link

tylim88 commented Nov 25, 2019

can we not rely on process.env.NODE_ENV but props instead?
because some of us don't want to see the console.log even in dev mode, it is extremely annoying

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

No branches or pull requests

9 participants