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

First pass external types support #41

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

curran
Copy link

@curran curran commented Dec 5, 2024

This PR introduces a new argument libraries that looks something like this:

// Example with React, Lodash, and D3
const libraries = {
  "react": "https://cdn.jsdelivr.net/npm/@types/react@18.2.0/index.d.ts",
  "lodash": "https://cdn.jsdelivr.net/npm/@types/lodash@4.14.195/index.d.ts",
  "d3": "https://cdn.jsdelivr.net/npm/@types/d3@7.4.0/index.d.ts"
};

This setup does not pull in arbitrary packages referenced in code, but rather forces the editor author to configure each library separately and pass the config into the CodeMirror extension. This solution is not as generic as something like https://github.com/val-town/deno-ata , but it's also a much lighter weight solution suitable for editing environments where a limited number of libraries are expected to be used. My main use case is for the VizHub editor, which primarily deals with D3 examples.

Closes #40

Closes #27

@curran
Copy link
Author

curran commented Dec 5, 2024

Current status:

image

image

  • Rough first pass - got it working to a point!
  • Full disclosure: done with the help of Aider + Claude
  • Misc code cleanup required on the PR - some of the changes may not be necessary
  • Works in Web Worker, not main thread
  • Highlights the need for more styling on the tooltip - too wide looks awkward

const system = createSystem(fsMap);
const compilerOpts = {};
const compilerOpts = {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced all these config changes are actually necessary. Could be AI slop. Need to circle back and see which options are strictly required for the type checking on external libs to work.

@tmcw
Copy link
Member

tmcw commented Dec 5, 2024

Thanks! But I don't think that this can close #27 or #40 - many NPM modules are going to import from other NPM modules, and for that to work, you need something like ATA. These examples of fully-bundled modules work, but they're too small a subset of the general TS ecosystem to be representative.

@curran
Copy link
Author

curran commented Dec 5, 2024

Fair enough. Yeah ATA is probably the way to go. Thanks for taking a look!

I recall I tried ATA in the past and ran into dependency hell, since ATA depends on typescript 4 as a peer dependency, and the project was using version 5.

https://github.com/microsoft/TypeScript-Website/blob/v2/packages/ata/package.json#L30C6-L30C16

I'm curious if you ran into anything like that, or if the https://github.com/val-town/deno-ata project could be used for non-deno packages. I suppose a next step for me is to attempt a "vanilla" ATA integration with codemirror-ts and see how that goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Type Definitions from NPM Packages Add recipe for remote node modules
2 participants