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

SWAT-972: Adding checks to ensure logger code can be imported in webpack.config #68

Closed
wants to merge 1 commit into from

Conversation

guptar8jan
Copy link

I need to require logger in webpack.config, to be able to log levels defined with logger. Doing so generates errors. I added couple of checks to ensure that logger can be imported when there is no document to work with.

@Velveeta
Copy link
Contributor

Velveeta commented May 2, 2017

I'm still unclear on where exactly this thing is throwing its error. Webpack is just a bundler, and shouldn't actually be executing any of the code in a Node runtime. So unless you're actually importing this package into some server-side code, I'm not sure why webpack is throwing this error. Can you elaborate on exactly where/when the error is manifesting, and which particular code is trying to use bv-ui-core?

@sirugh
Copy link
Contributor

sirugh commented May 2, 2017

The bv-ui-core is intended to be used in a browser. There are no guarantees it will work in node. As @Velveeta said, it shouldn't be executing in the runtime, so why/how/where is this error occurring? Rather than fix a symptom lets try to understand the cause.

@guptar8jan
Copy link
Author

@sirugh Issue is we are trying to import logger in webpack.config. @Velveeta suggested a workaround, I think that should work. Closing this PR.

@guptar8jan guptar8jan closed this May 2, 2017
@Velveeta
Copy link
Contributor

Velveeta commented May 2, 2017

I just finished working through it with her. They were using it as part of their webpack config file itself, so the DefinePlugin could set the logLevel throughout their application based on which webpack config was being run, which makes sense as a goal. I suggested they instead have their webpack config define their environment, and set up a log-level module that they can import to get that setting, something like this:

import logger from 'bv-ui-core/lib/logger';

let logLevel = logger.ERROR;

if (ENV === 'local') {
  logLevel = logger.DEBUG;
}

export default logLevel;

I think that's the route she's gonna take now, and that would put bv-ui-core's inclusion squarely within the app itself, and not in the webpack config, which tries to execute in Node.

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

Successfully merging this pull request may close these issues.

None yet

3 participants