solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.17k stars 918 forks source link

Cannot use istanbul ignore comments for generated code #2138

Open CreativeTechGuy opened 5 months ago

CreativeTechGuy commented 5 months ago

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):

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

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 commented 5 months ago

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.

CreativeTechGuy commented 5 months ago

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.