Alex - Thanks for the feedback. Interesting and intriguing observations.
The reason for getting the browsers object during render is, if we don't define that as a method, it might be evaluated during build time (say, webpack build). So I thought it'd be a good idea to get the object during render. Although now I think that we could do this only once in the constructor and not during each render.
The fact that electron is a browser totally skipped my mind. We should add that to the list of browsers definitely. I'm open to PRs :)
Distinction between browser version makes a lot of sense, say we want to put up a banner only in IE10 and not in IE11. Although we have to think through the syntax design as well. 'Coz the syntax of the component looks great now and I'm not sure how to add versions to it without complicating the use case. Gotta think through this.
I really appreciate you taking the time to go through the code and come back with areas of improvement. Thanks again!
Code doesn't execute during build time. Though it will execute during SSR. Doing it in constructor/render will make it execute during SSR which will fail. You can do it in componentDidMount and update state to make sure it only executes on client.
You're right. I meant the build time assignment evaluation that happens when we run minification as how uglify plugin does. Still, it wouldn't matter as we wrap all the browser detection as individual methods.
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
Alex - Thanks for the feedback. Interesting and intriguing observations.
The reason for getting the
browsersobject during render is, if we don't define that as a method, it might be evaluated during build time (say, webpack build). So I thought it'd be a good idea to get the object during render. Although now I think that we could do this only once in the constructor and not during each render.The fact that electron is a browser totally skipped my mind. We should add that to the list of browsers definitely. I'm open to PRs :)
Distinction between browser version makes a lot of sense, say we want to put up a banner only in IE10 and not in IE11. Although we have to think through the syntax design as well. 'Coz the syntax of the component looks great now and I'm not sure how to add versions to it without complicating the use case. Gotta think through this.
I really appreciate you taking the time to go through the code and come back with areas of improvement. Thanks again!
Code doesn't execute during build time. Though it will execute during SSR. Doing it in constructor/render will make it execute during SSR which will fail. You can do it in componentDidMount and update state to make sure it only executes on client.
You're right. I meant the build time assignment evaluation that happens when we run minification as how uglify plugin does. Still, it wouldn't matter as we wrap all the browser detection as individual methods.