Handle methods called on created instances

[jackbsteinberg]

In order to test certain methods for #34 I attempted to add a test that looks like this:

const d = new Document();
const elt = d.createElement('div');
elt.remove();

but the nodeInIDLSpecs function failed, because .getType() returned a strange error value and .getSymbol() on that value produced undefined. This leaves the above case unhandled.

[domenic]

This may be the same problem as #29.

The immediate error with nodeInIDLSpecs could be fixed by #38, but probably then we'd end up with #29?

[domenic]

Hmm, this may be a separate problem, actually. In particular I suspect this is happening due to the two-phase rewrite. First that gets translated to

import Document from "std:global/Document";

const d = new Document();
const elt = d.createElement('div');
elt.remove();

then we run the method transformer. However, at this point, ts-morph doesn't realize that Document generates instances of the built-in Document type; it's just some unknown constructor from some unknown module. So it doesn't know that d.createElement returns elements.

I'm not sure on the exact best solution here, but I'm currently thinking something like: whenever you replace a node, you need to do something like

newNode.originalType = nodeBeingReplaced.getType()

and then whenever we consult the type, we consult n.originalType || n.getType(). Maybe. Will try to think harder.

[jackbsteinberg]

I wonder if some of the confusion with the nesting is being caused by replacing parent nodes with text, messing with the chain of iteration.

I'll explore this a bit more, to see if I can fix the nesting issue trivially but only replacing nodes being evaluated.

[domenic]

Right, that is related to https://github.com/jackbsteinberg/get-originals-rewriter/issues/29#issuecomment-519704698's first bullet point.

[domenic]

Possibilities brainstormed today, that are probably better than my originalType idea:

• Generate type definitions for all the "std:global/*" modules, and feed them to TypeScript. Unsure exactly how to do this, but it seems pretty clean if we can make it work.

• Switch to a two-pass strategy that first discovers all nodes to replace, then replaces them. This will be hard because we'd need to work harder than we are currently to preserve node identity. E.g. currently we just do replaceWithText() for an entire call expression, including the arguments. We'd need to move the original argument nodes inside the new call expression, instead.

• Insert JSDoc comments as we do the replacement. Probably best done at the import site, i.e. so TypeScript knows the return value of each imported original. Best paired with a post-processing pass that strips them away afterward, to keep things clean.

[jackbsteinberg]

While I was adding tests I had a few fail for this issue, so I'll mark them down here so I can add them when this issue is getting resolved:

namespaces test:

// namespaces.input.js, lines 6-7
const layer = css;
layer.escape();

// namespaces.output.js, line 9
CSS_escape();

property-access test:

// property-access.input.js, lines 4-5
const ol = str.length;
const arr = str.split('2');

// property-access.output.js, lines 2, 6-7
import { split as String_split } from "std:global/String";
...
const ol = str.length;
const arr = Reflect_apply(String_split, str, ['2']);

replace-constructors-alias test:

// replace-constructors-alias.input.js, line 3
const elt = a.createElement('div');

// replace-constructors-alias.output.js, lines 1-2, 5
import { apply as Reflect_apply } from "std:global/Reflect"
import Document, { createElement as Document_createElement } from "std:global/Document";
...
const elt = Reflect_apply(Document_createElement, a, ['div']);