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

Core: improve warning for incorrect module hook usage #1648

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

chriskrycho
Copy link
Contributor

Add the names of the relevant modules so the message is more actionable for users who encounter it.

Fixes #1647

@Krinkle, this is failing tests locally (and presumably on CI here!) for CLI Main > mapped trace with native source map, presumably because it needs the fixture regenerated. I’m happy to update the fixture given guidance to that end!

Add the names of the relevant modules so the message is more actionable
for users who encounter it.

Fixes qunitjs#1647
@Krinkle
Copy link
Member

Krinkle commented Aug 25, 2021

@chriskrycho Seems to pass here. Mind sharing the failure details? My guess it's a false negative from a newer Node version emitting slightly different source maps or perhaps new internal stack frame somewhere.

@chriskrycho
Copy link
Contributor Author

Ah, it is indeed likely a newer version. I’m running Node 14 LTS locally. (I have gotten spoiled by Volta keeping these things in sync for me over the last couple years!)

@chriskrycho
Copy link
Contributor Author

I’ll add the failure details in a new issue when I’m back at my work laptop tomorrow!

@chriskrycho
Copy link
Contributor Author

The error message report:

not ok 52 CLI Main > mapped trace with native source map
  ---
  message: failed
  severity: failed
  actual  : |+
    TAP version 13
    ok 1 Example > good
    not ok 2 Example > bad
      ---
      message: failed
      severity: failed
      actual  : false
      expected: true
      stack: |
            at /qunit/test/cli/fixtures/sourcemap/source.min.js:1:133
                -> /qunit/test/cli/fixtures/sourcemap/sourcemap/source.js:7:10
      ...
    1..2
    # pass 1
    # skip 0
    # todo 0
    # fail 1
  expected: |+
    TAP version 13
    ok 1 Example > good
    not ok 2 Example > bad
      ---
      message: failed
      severity: failed
      actual  : false
      expected: true
      stack: |
            at /qunit/test/cli/fixtures/sourcemap/sourcemap/source.js:7:10
      ...
    1..2
    # pass 1
    # skip 0
    # todo 0
    # fail 1
  stack: |
        at Object.<anonymous> (/Users/ckrycho/dev/qunitjs/qunit/test/cli/cli-main.js:401:13)
        at processTicksAndRejections (internal/process/task_queues.js:93:5)
  ...

@Krinkle
Copy link
Member

Krinkle commented Aug 26, 2021

Interesting. We run Node 14 in CI as well, and there Node.js reports source-mapped traces as input.min -> mapped.source instead of only mapped.source. We normalize them in execute.js. Feel free to poke around if it interests you. Otherwise, I might get around to it next week when releasing this.

@Krinkle Krinkle merged commit 93eb8a8 into qunitjs:main Aug 26, 2021
@chriskrycho chriskrycho deleted the better-module-error branch August 26, 2021 15:14
@chriskrycho
Copy link
Contributor Author

Curious. Unfortunately I will not have time to fix it, as I have other open-source things which need to be a higher priority, but thank you in any case for the quick response and turn-around here! Very much appreciated.

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

Successfully merging this pull request may close these issues.

Report affected modules for "hook called from wrong module" warning
2 participants