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

Add deprecation warning to parseQuery #57

Merged
merged 1 commit into from
Feb 10, 2017
Merged

Conversation

jhnns
Copy link
Member

@jhnns jhnns commented Feb 9, 2017

This deprecation warning is intended to alarm loader authors that passing an object to parseQuery and then modifying it might lead to unintended behavior.

#56

index.js Outdated
"loaderUtils.parseQuery() just received a value type of " + typeof query +
" which can be problematic, see https://github.com/webpack/loader-utils/issues/56" + os.EOL +
"parseQuery() will be replaced with getOptions() in the next major version of loader-utils."))(),
query;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really have to be formatted this way??

The , operator is difficult to understand and I think it's better to use { ...; ...; } here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't using deprecate this way will report the message on every call instead of only once?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right with both. I've changed it

This deprecation warning is intended to alarm loader authors that passing an object to parseQuery and then modifying it might lead to unintended behavior.

#56
@jhnns jhnns merged commit 34437d6 into master Feb 10, 2017
@jhnns jhnns deleted the add/parse-query-warning branch February 10, 2017 14:47
This pull request was closed.
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.

2 participants