Recover from incomplete ESM using acorn-loose - #528
Conversation
🦋 Changeset detectedLatest commit: 4d350dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi! It seems you removed the template which we require. Here are our templates (pick the one you want to use and click *raw* to see its source): I won’t send you any further notifications about this, but I’ll keep on updating this comment, and hide it when done! Thanks, |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
==========================================
+ Coverage 92.43% 92.91% +0.48%
==========================================
Files 13 13
Lines 1982 2117 +135
==========================================
+ Hits 1832 1967 +135
Misses 150 150 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I’m curious to hear what others, mostly @remcohaszing for example, think! I am not that involved with this project. But we’ve had conversations before about the impossibility of some of this 🤔
The arguments you use, would also mean that it’s a great idea to do for expressions. Why not?
MDX is a programming language. There are errors. Same with Rust or JS. A syntax error is a real error. We do want crashes when people have syntax errors in JS/Rust/MDX/etc. Have you looked into putting the code in |
|
Yes, I have been meaning to look into this, prioritize it even! But the timing was inconvenient for me and this got pushed down my todo list. I appreciate the ping! This is a really cool feature, but it also need thorough manual testing. I’ll dedicate some proper time this week. |
remcohaszing
left a comment
remcohaszing
left a comment
There was a problem hiding this comment.
It’s great that this allows the language service to process files further. But these are still syntax errors that need to be propagated to the editor.
In virtual-code.js, we need to iterate the estree for all MDX specific node types. If we find a dummy node using the isDummy function exported from acorn-loose, we should report it as a VFileMessage() on the VirtualMdxCode.
| "acorn": "^8.14.0", | ||
| "acorn-jsx": "^5.3.0", | ||
| "acorn-loose": "^8.5.2", |
There was a problem hiding this comment.
We use loose ranges in the unified ecosystem.
| "acorn": "^8.14.0", | |
| "acorn-jsx": "^5.3.0", | |
| "acorn-loose": "^8.5.2", | |
| "acorn": "^8.0.0", | |
| "acorn-jsx": "^5.0.0", | |
| "acorn-loose": "^8.0.0", |
|
|
||
| assert.ok(code instanceof VirtualMdxCode) | ||
| assert.ifError(code.error) | ||
| assert.ok(code.embeddedCodes[0].mappings.length > 0) |
There was a problem hiding this comment.
Could you use the same type of assertion as other tests?
assert.deepEqual(code, [
// …
])I find it helpful to have a full picture of what the virtual code looks like.
| assert.ok(code.embeddedCodes[0].mappings.length > 0) | ||
| // Verify the generated JS contains both the import and export | ||
| const jsText = code.embeddedCodes[0].snapshot.getText( | ||
| 0, | ||
| code.embeddedCodes[0].snapshot.getLength() | ||
| ) | ||
| assert.ok(jsText.includes('import {Planet} from "./Planet.js"')) | ||
| assert.ok(jsText.includes('export const greeting = "hello"')) |
There was a problem hiding this comment.
Could you use the same type of assertion as other tests?
assert.deepEqual(code, [
// …
])I find it helpful to have a full picture of what the virtual code looks like.
| // The key assertion: embedded code has non-empty mappings despite broken ESM. | ||
| // Before acorn-loose recovery, this would have empty mappings ([]). | ||
| assert.ok(code.embeddedCodes[0].mappings.length > 0) |
There was a problem hiding this comment.
Could you use the same type of assertion as other tests?
assert.deepEqual(code, [
// …
])I find it helpful to have a full picture of what the virtual code looks like.
cf1a2d6 to
4973d36
Compare
4973d36 to
bfc2182
Compare
| "overrides": { | ||
| "vscode-markdown-languageservice": "0.5.0-alpha.11", | ||
| "vscode-languageserver": "9.0.1", | ||
| "vscode-languageserver-protocol": "3.17.5", | ||
| "vscode-languageserver-types": "3.17.5" | ||
| }, |
There was a problem hiding this comment.
Without them,test-types fails in CI.
3 participants
Initial checklist
Description of changes
Closes #267
Problem
When users type incomplete ESM (e.g.
export const x =orimport { } from) in an MDX file, the strict acorn parser fails and the language service loses all IntelliSense — completions, diagnostics, and hover information disappear for the entire file until the syntax is corrected.This is a poor editing experience because ESM is always incomplete while the user is actively typing it.
Approach
Introduce a "hybrid acorn" strategy that uses
acorn-loosefor ESM parsing and the strictacornparser (with JSX) for expressionsLong-term perspective
This is a minimal, low-risk first step. Possible future improvements include working with
micromark-extension-mdxjs-esmto support pluggable error recovery, reducing the need for patching at the language-service level.