Fix validate-jsdoc-codeblocks rule to run diagnostics using latest file contents

[som-sm]

Problem

To run TS diagnostics on JSDOC codeblocks, we simply create the virtual environment once and then use a single virtual file to get all the diagnostic messages.

The problem is that after the virtual environment is created, it does not detect any changes to files on disk. So, if a JSDoc codeblock imports something from a real file, the environment always resolves that file as it existed when the virtual environment was created, ignoring any updates made afterward.

And, this makes the DX really bad, because if we make any changes, the diagnostics don't update accordingly. We must save the changes and restart the linter to get diagnostics based on updated content.

demo-1.mov

Fix

Fixing this wasn't as straightforward as I thought.

Attempt 1

I initially thought that creating the environment inside the create(context) block would fix the issue. However, this approach doesn't work because the create(context) block runs whenever a file changes, not when a file is saved. So, even though a new environment is created every time a file changes, that new environment does not include any unsaved changes. In other words, the environment only ever sees the last saved version of a file.

This leads to a confusing situation. When you save a file, the environment still reflects the previous saved state, because the create(context) block does not run after the save. To work around this, you'd have to save your changes and then make an additional modification just to trigger the create(context) block again, allowing the environment to finally pick up the recently saved changes.

video-2.mp4

Attempt 2

Next, I tried updating the contents of the file that changed:

env.updateFile(context.filename, context.sourceCode.getText());

This immediately produced a runtime error:

> npm run test:xo -- source/remove-prefix.d.ts

Error: Error while loading rule 'type-fest/validate-jsdoc-codeblocks': Did not find a source file for /Users/som/Developer/projects/type-fest/source/remove-prefix.d.ts

The error message indicated the virtual environment wasn't aware of the file we tried to update. That seemed odd, because we've codeblocks that successfully import stuff from that file, so the environment should know about it. The catch was that the environment only becomes aware of a real file if it's imported by a virtual file.

Attempt 3

So, if we move the context.filename update to run after we update the virtual codeblock file (example-codeblock.ts), things should work?

env.updateFile(FILENAME, code);
env.updateFile(context.filename, context.sourceCode.getText());

And, they do. This works because once we've updated the virtual file with the contents of a codeblock, the virtual environment becomes aware of the context.filename file, assuming the codeblock imports the context.filename file.

This approach works well and the DX is great. We get live updates as soon as we change something, without having to save.

demo-3.mov

Note

We still need a try...catch around the context.filename update because there's no guarantee that the codeblock will import context.filename correctly, and we definitely don't want the lint rule to explode because of that.
For example, if we add a new type Concat that has not yet been exported from index.d.ts, the import inside the codeblock will fail, so the virtual environment will not get to know about this Concat file. So, updating it will cause an error, and that error will suppress all the other helpful error messages:

env.updateFile(FILENAME, code);
try {
  env.updateFile(context.filename, context.sourceCode.getText());
} catch {
  // Ignore
}

With the try...catch block in place, we will get the other helpful errors:

Still, it felt like we can do better, because right now we are updating context.filename for every single codeblock, which is unnecessary.

Solution

So, we simply move the update logic to the beginning of the create(context) block:

create(context) {
	try {
		env.updateFile(context.filename, context.sourceCode.getText());
	} catch {
		// Ignore
	}
	
	...

This works fine because the first time it runs, the update throws, and we simply catch and ignore it. On subsequent runs, if a codeblock imported context.filename in the previous run, the virtual environment would now be aware of context.filename, and the update would succeed.

And if none of the codeblocks imported context.filename, the update would fail again, but that'd be fine too because if none of the codeblocks imported context.filename, it means there wasn't a need to update context.filename at the first place.

[som-sm]

@sindresorhus It's a really small change, I've just added a huge description for my own reference, feel free to skip it.