Extracting errors during build does not support string templates

[Weffe]

Current Behavior

Running --extractErrors on the build script will fail if you use a string template as the invariant message instead of normal string quotes.

Example index.ts:

// index.ts
import invariant from 'tiny-invariant';

invariant(false, `test`);

Results in:

> tsdx build --extractErrors

✓ Creating entry file 4 secs
(at position 1 plugin) Error: Unsupported type TemplateLiteral
Error: Unsupported type TemplateLiteral
    at Object.evalToString (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/tsdx/dist/errors/evalToString.js:13:19)
    at exit (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/tsdx/dist/errors/extractErrors.js:70:64)
    at NodePath._call (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/path/context.js:76:18)
    at NodePath.call (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/path/context.js:48:17)
    at NodePath.visit (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/path/context.js:117:8)
    at TraversalContext.visitQueue (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitSingle (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/context.js:108:19)
    at TraversalContext.visit (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/context.js:192:19)
    at Function.traverse.node (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/index.js:114:17)
    at NodePath.visit (/home/rogelio/Desktop/localdev/shopify-checkout-step-manager/node_modules/babel-traverse/lib/path/context.js:115:19)

I think this has to do with the evalToString method in src/errors/evalToString.ts. It fails to evaluate TemplateLiteral expressions and thus fails to extract any errors.

Expected behavior

I expected this to extract the errors as it normally would if I were to use plain string quotes.

Suggested solution(s)

Having to support string templates in the invariant message might be tricky. This is due to not knowing the literal values for the template variables that needs to be interpolated during runtime.

For example,

// index.ts
import $ from 'jquery';
import invariant from 'tiny-invariant';
import api from 'google-api';

const query = $('.search-input').text();
const results = api.getResults(query);

invariant(results.isNotEmpty, `This query has no results -> ${query}`);

^ Impossible to guess what query is when extracting the error message.

I see two possible solutions to this:

1. Highlight in the docs that string templates as the message for the invariant function is not supported. Rather, use plain string quotes.

2. Evaluate string templates, but fail if we cannot properly interpret the literal values of any template variables.

Having support for string templates and error extraction is nice because I use them for ease of use with multi-line comments. But, if the work required to support string templates is too much then I do think the next best thing is to go with option 1 and highlight the limitations of error extraction in the docs.

Your environment

Software	Version(s)
TSDX	0.11.0
TypeScript	3.7.2
Browser
npm/Yarn	npm v6.12.0
Node	10.15.0
Operating System	Linux Mint 19.1
tiny-invariant	1.0.6

[swyxio]

we can at least document this as a first measure. PR welcome. i agree supporting string templates will be tricky.

[jaredpalmer]

Wow TIL!!

Background
I ripped extract errors out of React core and remember that they don’t use string templates. At the time, I didn’t really think about it much and just thought it was because React source code is weird AF. This was pretty annoying actually when I PR’d a React error message once. Anyways, they use simple multi line concatenation with + and dub quotes IIRC.

Thoughts
I have no idea how to support string interpolation, but we should ideally document and better yet fail if they are used.

[Weffe]

Definitely agree on adding some documentation on the limitation.

In order to support this with minimal dev work, we could only allow Template Literals with 2 conditions:

1. The template literal contains NO interpolated values
2. The template literal contains SOME interpolated values that are only string types

I can see myself combining string error messages together and others might do the same too. Supporting other interpolated values like Functions or Arrays/Objects could get tricky since we'd have to do some extra work (e.g. awaiting an async function? Or JSON.stringify arrays/objects which could have circular deps?).

What do you think?

[agilgur5]

Somewhat unrelated, but could this feature be split out of core and into its own library?

AFAIK, it seems like it can be, there's only a handful of internal references. It's a very advanced feature and would be easier to maintain that way as well as usable outside of TSDX. #184 would probably be easier that way too.