bug: invalid error callstacks and coverage reports in tests for components.

[paladin80]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

2.10.0 - 2.11.0

Current Behavior

When running tests in stencil, any error callstacks and coverage report don't match the source code.

Expected Behavior

Error callstacks should point exactly to the line of TS code, where the error happened. Jest coverage report files should properly highlight source files.

Expected behavior was fullfilled in older stencil versions.

Steps to Reproduce

For call stacks:

1. npm init stencil, choose component. Or just download the stencil-component-starter.
2. npm install
3. Open utils.ts and add a throwing code to line 2. Example: '1'.d();
4. npm test
5. Note the error output of tests: TypeError: "1".d is not a function
   at Object.format (src/utils/utils.ts:308:7)
   at Object. (src/utils/utils.spec.ts:15:20)
6. The callstack points to line 308 of the utils.ts, but this file has less than 10 lines of code.
7. The callstack points to line 15 of utils.spec.ts, which is an empty line.

For coverage:

1. As in previous steps, install stencil-component-starter.
2. Open utils.ts and add a false if statement with some code:
   if (1 == 2) {
   first = middle;
   }
3. Run stencil test --spec --coverage
4. Note that created coverage report has invalid highlighting for the utils.ts
   

Code Reproduction URL

https://github.com/paladin80/stencil-component-starter

Additional Information

I think that generated source maps don't work in tests.

[smndhm]

I'm having the same issue.
When downgrading to @stencil/core@2.9 and removing Jest cache jest --clearCache I'm not having it anymore.

[paladin80]

The issue appeared in the version 2.10.0. And is still present in 2.11.0

Jest cache is preventing to clearly discover the problem. So please remove the "jest" folder from your %tmp% directory when testing the versions.

[paladin80]

OS used: Windows 10 x64
Node used: 14.18.1 and 17.1.0 (same result)

[johnjenkins]

just discussed this in stencil slack (here) - turns out the issue was unknowingly introduced within here as it effects this line < the new encoding should not run on an inlined sourcemap .. data:application/json;charset=utf-8 will become data%3Aapplication%2Fjson%3Bcharset%3Dutf-8%2C for example.
To test, within the stencil repo, jest --clearCache and run npm run test - take note of the coverage report - the file line numbers don't seem to relate to much.
Change the offending line to results.code = results.code.slice(0, sourceMapComment) + '//# sourceMappingURL=' + sourceMapInlined;, recompile, jest --clearCache and run npm run test. Coverage report looks very different. Line numbers relate to obvious blocks of code.

[paladin80]

Thank you @johnjenkins. In the slack I just said my hypothesis that I got from comparing the code in branches. I had no time to properly test and verify it.

Can you create a pull request for this issue?

[rwaskiewicz]

👀

[rwaskiewicz]

Thanks @paladin80 for the super detailed reproduction case and @johnjenkins for debugging this! Both were super helpful and used directly in #3163, which should fix this issue.

[paladin80]

Hi @rwaskiewicz. Thank you for the quick response.
The fix works, but now the code doesn't call the encodeFilenameToRfc3986() function. It looks like a rollback to an older status quo. Maybe there's another solution to keep both the URL RFC-compliant and don't lose the compatibility with the Node.

[rwaskiewicz]

@paladin80 Hmm it looks like '+', '/', and '=' are all reserved characters in RFC-3986 that can also be part of a valid Base 64 encoding. Let me rework this real quick...

[paladin80]

I didn't dig into it yet, but I think that we shouldn't encode the part data:application/json;charset=utf-8;base64, since it is a protocol definition, and encode everything else. Testing is needed.

EDIT: On the other side, the current url is RFC-2397 compliant, so maybe there's no need to do additional encoding.

[rwaskiewicz]

I spent some additional time on my PR, added tests, then tested everything manually and think everything is working in the PR as expected. #3163 has been merged, and will be in the next Stencil release (v2.12.0)

[rwaskiewicz]

This fix has been included in the v2.12.0 release. I'm going to close this issue, however if the problem reappears, please feel free to open a new issue!