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

Cannot use istanbul ignore comments for generated code #2138

Open
CreativeTechGuy opened this issue Apr 18, 2024 · 2 comments
Open

Cannot use istanbul ignore comments for generated code #2138

CreativeTechGuy opened this issue Apr 18, 2024 · 2 comments
Labels
compilation enhancement New feature or request

Comments

@CreativeTechGuy
Copy link

CreativeTechGuy commented Apr 18, 2024

Describe the bug

When using Solid and testing our app with coverage reporting enabled (powered by istanbul/nyc), there are some branches/functions which are uncovered, but they are not branches which exist in our code, instead they are generated by Solid. Because these code paths are generated, we cannot place a /* istanbul ignore next */ comment before the appropriate section because it doesn't exist anywhere in our source.

A few cases where this occurs (I'm sure there's others):

  • Show/Match components - If the match is never used, there's an unused function because of the get children() method which got generated
  • Refs - It generates a ternary with typeof _ref$ === "function" as the condiiton. Which in most cases will always be true/false so that means there's a branch of the code which never gets hit.

All of these result in an extra uncovered function here, an extra uncovered branch there. And there's no way we can add the appropriate comments before this code to ignore it.

Your Example Website or App

https://playground.solidjs.com/anonymous/7b917606-9c75-4695-b669-accb486afc92

Steps to Reproduce the Bug or Issue

N/A

Expected behavior

When Solid is generating the runtime code, it should relocate the comments appropriately so they keep the same meaning that they have in the source.

Source:

{/* istanbul ignore next */}
<Match when={condition()}>
    <div>Hello</div>
</Match>

Generated:

_$createComponent(Show, {
  get when() {
    return condition();
  },
  /* istanbul ignore next */
  get children() {
    return _tmpl$();
  }
})

Source:

let ref;
return <div ref={ref}>Hello</div>

Generated: (This should always add the appropriate ignore comments since only one branch will ever be hit.)

let ref;
return (() => {
  var _el$ = _tmpl$();
  var _ref$ = ref;
  /* istanbul ignore next */
  typeof _ref$ === "function" ? _$use(_ref$, _el$) : ref = _el$;
  return _el$;
})();

Screenshots or Videos

No response

Platform

  • OS: N/A
  • Browser: N/A
  • Version: 1.8.16

Additional context

It's also impossible currently to write our own plugin to do this since comments will get totally deleted during compilation when placed in certain spots of the JSX.

For example, this input:

function Counter() {
  let ref;
  /* istanbul ignore next -- 1 */
  return (
    <>
      {/* istanbul ignore next -- 2 */}
      <div ref={ref}>Hi</div>
    </>
  );
}

Gets transformed into: (the second comment is just totally lost so we cannot even attempt to move it with our own plugin after Solid is done compiling)

function Counter() {
  let ref;
  /* istanbul ignore next -- 1 */
  return (() => {
    var _el$ = _tmpl$();
    var _ref$ = ref;
    typeof _ref$ === "function" ? _$use(_ref$, _el$) : ref = _el$;
    return _el$;
  })();
}

This is a unique problem with Solid which doesn't exist with many other frameworks because Solid generates a lot of code rather than just calling functions from the Solid library. If these same bits of code were instead unconditional function calls to code in node_modules, this wouldn't be an issue.

@ryansolid
Copy link
Member

Yeah this is a tricky one. Source maps have also been challenging. We've made some improvements but due to the need for function wrappers/special handling of ref those things need to be inline. Like above _$use is the helper function but to keep references we can't pull that all in.

It's an interesting problem in that tooling that operates on source (ie not compiled code like in IDEs like TS etc) aren't going to have this problem. I wonder how this works in stuff like Svelte/Vue. Admittedly other than trying to keep references in compilation we generally don't go out of our way and try to let Babel do its thing.

@ryansolid ryansolid added the enhancement New feature or request label Apr 19, 2024
@CreativeTechGuy
Copy link
Author

Oh yeah I definitely have had trouble with source maps when debugging before too now that you mention it.

Is there any solution to something like this? I'm trying to get my organization to buy in to rewriting our app in Solid and it's going to be a tough sell if there's no workaround for this (aside from just ignoring it).

But you are right, it's not a single tool problem but something much bigger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compilation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants