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

Report affected modules for "hook called from wrong module" warning #1647

Closed
chriskrycho opened this issue Aug 25, 2021 · 5 comments · Fixed by #1648
Closed

Report affected modules for "hook called from wrong module" warning #1647

chriskrycho opened this issue Aug 25, 2021 · 5 comments · Fixed by #1648
Labels
Component: Core For module, test, hooks, and runner. help welcome Type: Enhancement

Comments

@chriskrycho
Copy link
Contributor

chriskrycho commented Aug 25, 2021

Currently, QUnit reports that a hook is called from the wrong module, but doesn't say which modules are affected:

qunit/src/module.js

Lines 114 to 118 in e457e29

if ( config.currentModule !== module ) {
Logger.warn( "The `" + hookName + "` hook was called inside the wrong module. " +
"Instead, use hooks provided by the callback to the containing module. " +
"This will become an error in QUnit 3.0." );
}

—even though the information is attached to the module:

name: moduleName,

This makes it quite difficult to figure out where the issue actually comes from. I'm happy to open a PR for this!

@Krinkle Krinkle added Component: Core For module, test, hooks, and runner. help welcome Type: Enhancement labels Aug 25, 2021
@Krinkle
Copy link
Member

Krinkle commented Aug 25, 2021

@chriskrycho Sounds good to me. Thanks!

@chriskrycho
Copy link
Contributor Author

PR inbound later today or tomorrow!

chriskrycho added a commit to chriskrycho/qunit that referenced this issue Aug 25, 2021
Add the names of the relevant modules so the message is more actionable
for users who encounter it.

Fixes qunitjs#1647
@chriskrycho
Copy link
Contributor Author

@Krinkle I've opened #1648 for this.

@Krinkle
Copy link
Member

Krinkle commented Aug 25, 2021

@chriskrycho Great.

Meanwhile, if I may ask, did you welcome this warning while writing a new test, or did it find you as a surprise after upgrading QUnit? This is a brand new warning we added, so we're certainly open to feedback or use cases you may have, which could influence how we proceed with this in QUnit 3.

@chriskrycho
Copy link
Contributor Author

I noticed it in a large app (LinkedIn) when working on other tests. I think the warning is good; the only issue was this one. As you can imagine, when you have tens of thousands of tests, tracing back to individual modules is tricky without more info!

Krinkle pushed a commit that referenced this issue Aug 26, 2021
Add the names of the relevant modules so the message is more actionable
for users who encounter it.

Fixes #1647.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core For module, test, hooks, and runner. help welcome Type: Enhancement
Development

Successfully merging a pull request may close this issue.

2 participants