Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix auto-imports #491

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix auto-imports #491

wants to merge 2 commits into from

Conversation

remcohaszing
Copy link
Member

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

This change adds support for auto-imports.

The virtual code now contains an empty import of the JSX runtime. This import was chosen, because it must exist anyway. This is immediately followed by an empty code mapping, meaning TypeScript always has a place to insert auto-imports.

Since JSX components can be injected, they are sometimes prefixed with _components. in the virtual code. To support auto-import completions, an additional mapping is now made to an expression containing merely the identifier. As a result, the editor now shows auto-import completions, unless MDXProvidedComponents is defined. I don’t know why the existence of MDXProvidedComponents matters, but this probably matches the expectation of users anyway.

The auto-imports will not be followed by a blank line. This can lead to a syntax error in case no other imports exist yet. This is not ideal, but easy and straight-forward to resolve manually.

Closes #452

This change adds support for auto-imports.

The virtual code now contains an empty import of the JSX runtime. This
import was chosen, because it must exist anyway. This is immediately
followed by an empty code mapping, meaning TypeScript always has a place
to insert auto-imports.

Since JSX components can be injected, they are sometimes prefixed with
`_components.` in the virtual code. To support auto-import completions,
an additional mapping is now made to an expression containing merely the
identifier. As a result, the editor now shows auto-import completions,
unless `MDXProvidedComponents` is defined. I don’t know why the
existence of `MDXProvidedComponents` matters, but this probably matches
the expectation of users anyway.

The auto-imports will not be followed by a blank line. This can lead to
a syntax error in case no other imports exist yet. This is not ideal,
but easy and straight-forward to resolve manually.

Closes #452
@remcohaszing remcohaszing added 🐛 type/bug This is a problem 🗄 area/interface This affects the public interface 👶 semver/patch This is a backwards-compatible fix 🙆 yes/confirmed This is confirmed and ready to be worked on 👍 phase/yes Post is accepted and can be worked on labels Jan 3, 2025
Copy link

changeset-bot bot commented Jan 3, 2025

🦋 Changeset detected

Latest commit: f7e9713

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@mdx-js/language-service Patch
@mdx-js/language-server Patch
@mdx-js/typescript-plugin Patch
vscode-mdx Patch

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

Copy link
Contributor

github-actions bot commented Jan 3, 2025

Hi! This is accepted and can go somewhere!

Team: please review this PR and use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 👍 phase/yes Post is accepted and can be worked on 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on
Development

Successfully merging this pull request may close these issues.

Auto-import IntelliSense of React components missing
1 participant