Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

config breaks jest inline snapshots due to ts-node/register #98

Open
G-Rath opened this issue Feb 15, 2020 · 3 comments
Open

config breaks jest inline snapshots due to ts-node/register #98

G-Rath opened this issue Feb 15, 2020 · 3 comments

Comments

@G-Rath
Copy link
Contributor

G-Rath commented Feb 15, 2020

import HelloWorld from '@src/commands/hello_world';

describe('fmt', () => {
  it('writes the results to disk', async () => {
    try {
      await HelloWorld.run([]);
    } catch (e) {
      console.log(e);
    }

    expect({}).toMatchInlineSnapshot();
  });
});

The above will fail in a typescript project due to config setting up tsNode.register. This is because Jest sees there's already a transformer for TypeScript, and so doesn't apply its own transformer.

This is "fine", except that the output of ts-node doesn't match the output of Babel (which is what jest uses); when writing inline snapshots, jest looks for all toMatchInlineSnapshot matchers by walking the ast as provided by Babel.

As such, it fails to find the snapshot matcher because while Babel is walking the AST, it didn't do the transformation, so the line numbers are different and it never finds the matcher.

In general, @oclif/config probably shouldn't be using ts-node like this, as it's not really needed: it's encouraging users to ship TS files when they should be compiled, and this behaviour can be replicated in userland by calling tsNode.register in your bin script, or by passing it to node as part of the call (for when testing, which I assume is why this feature was added).

Additionally, it's probably going to cause oclif some pain when ESM modules land, as it'll likely force people to stick on commonjs.

Removing ts-node would solve a number of issues that have been opened around oclif packages.

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 15, 2020

@RasPhilCo @jdxcode I've confirmed that removing ts-node from config while leaving the rest of registerTSNode & surrounding code is painless, with the previous functionality being restorable with the following package.json script:

"cli": "node -r ts-node/register bin/<mycommand>",

Let me know if you're open to a PR

@RasPhilCo
Copy link
Contributor

RasPhilCo commented Jun 8, 2020

@G-Rath open to this idea. Initial thoughts: 1) Maybe the registering can be handled in a bin/dev shim. 2) We likely need to still back-support for sometime but oclif generators could start producing a new shim

@G-Rath
Copy link
Contributor Author

G-Rath commented Jun 8, 2020

If I understand you correctly, those should be fine - the problem is about that the registering currently exists in oclif instead of userland, so you can't opt-out and have to carry the dependency with you.

So (again if I'm understanding you correctly) having the generator generate a new bin/dev script for typescript projects that does the registering would be fine since I think that's a cool default to give them and it lets the user remove it if they decide they won't want the ts-node dependency in their tree.

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

No branches or pull requests

2 participants