Svelte2tsx Fix SourceMapping range endings

[pushkine]

As requested by @dummdidumm, this PR is currently only a draft "POC" demonstration of the fix on a subset of the codebase (actions).

Issue

For what I suppose is an unintuitive yet implicit rule of SourceMapping, it appears that looking up the origin of a range (such as an identifier) implies looking up the first character and the character after the last one in the range.

In other words svelte2tsx's sourcemap is wrong whenever the character following a range in tsx does not exactly map to the character following that same range in svelte.

Example

Input

<input use:foo />

Output

<input•{...__sveltets_ensureAction(foo(__sveltets_mapElementTag('input')))}•/>↲

Mapped characters

<input•                            foo                                     •/>↲

Issue : looking up foo returns fo
To lookup the original position of foo, the vscode language server queries the positions of f and (
( is not mapped, its position falls back to the last mapped character in that line, o
In the end, looking up the position of the identifier foo resolves into fo

Current workaround

The Svelte language server currently hacks around that issue by monkey patching positions after the fact

language-tools/packages/language-server/src/lib/documents/DocumentMapper.ts

Lines 220 to 228 in 0ac9826

	// Range may be mapped one character short - reverse that for "in the same line" cases
	if (
	originalRange.start.line === originalRange.end.line &&
	range.start.line === range.end.line &&
	originalRange.end.character - originalRange.start.character ===
	range.end.character - range.start.character - 1
	) {
	originalRange.end.character += 1;
	}

It works but the output is still wrong, meaning that other tools relying on svelte2tsx must monkey patch too.
Unfortunately the Svelte Typescript Plugin #842 cannot access internal typescript ranges, only individual positions.

Solution

This issue occurs in many parts of the codebase. MagicString does not provide a solution, and it's not evident that it would, perhaps because that library is focused on strings, and this would require it to process AST Nodes.

This PR introduces tagged template string helpers, kind of similar to Rich-Harris/code-red

const node = handle_subsets(magic_string, svelte_node);

/**
 * New API to deconstruct Svelte AST Nodes
 * 
 * Svelte sometimes only returns snippets as raw strings
 * Those must be first normalized into real Nodes
 */
const [action] = node.deconstruct`use:${attr.name}`;

/** 
 * Note that the strings in between nodes are regular expressions
 * Handy for Svelte quirks such as quoted expressions
 */
const [action, expression] = node.deconstruct`use:${attr.name}=["']?{${attr.expression}}["']?`;

/**
 * New API to edit snippets
 * 
 * Abstract layer above MagicString that handles AST Nodes differently
 * Edits the whole subset, and automatically handles mappings for every ${Node}
 */
node.edit`{...__sveltets_ensureAction(${action}(__sveltets_mapElementTag('${parent.name}')))}`;

/**
 * This PR has the side effect of making svelte2tsx much more concise and readable
 * This 6-line codeblock is nearly all it takes to handle actions now!
 */
node.edit`{...__sveltets_ensureAction(${action}(__sveltets_mapElementTag('${parent.name}'),(${expression})))}`;

Here is the output for the above example, notice the character after foo in tsx being mapped to the character after foo in the original code

[dummdidumm]

The concept is neat! Especially the readability part is a huge plus. I see some potential drawbacks though:

• It may hurt performance since each string now has to be analyzed (might be irrelevant but it needs a check)
• For deconstruction, I need to essentially know how the string (or parts of it) looked like and reconstruct it. I'm not sure that this can be done for all nodes or if it is too brittle. On the other hand, we do essentially the same nowadays, too, using the start/end numbers. So this might not be a real drawback.
• This abstraction uses MagicString in a specific way, always using the same methods in the same cases. This might not work because of the way we go through the code. In some cases we need to do appendLeft etc instead of prependLeft, and we definitely need overwrite(start, end, '', { contentOnly: true } in some places instead of remove. Losing the ability to choose will surely hurt/not work in some places

@jasonlyu123 thoughts?