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

WIP Exploration of JSR type aquisition #7

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

Conversation

zephraph
Copy link

@zephraph zephraph commented Oct 14, 2024

Note

This isn't intended to be merged. It's just going to be a record of my exploration.


I've started digging deeper into JSR type acquisition and what would be required to support JSR types. I need to dig more into what Deno's doing in their LSP...

I explored how @nandorojo was constructing his examples in #5. With the change I have below I've been able to get my webview package running successfully.

import { createWebView } from "jsr:@justbe/webview"

I was hoping it'd be just that simple (wouldn't that be a treat?). That works in some cases with relatively simple type declarations. I've got several other things I tired: switching how I handled declarations depending on if the declaration file was a global declaration or a module declaration (determined by if there's a top level export), separating out the imports and exports and only wrapping the exports in a declaration.

I need to spend more time with it. The biggest thing I think would be helpful is to be able to grab the diagnostics of these generated type files. Downstream type checking is failing so the compilers bail at a certain point and we get whatever it seems to have made it through. There's got to be a better way to get signal about why these modules aren't loading. I'm going to look at it more this week. @orta if you have thoughts, I'd love to hear them!

@orta
Copy link

orta commented Oct 14, 2024

I don't think there is a way to indicate that without knowing a type you can check for after you've done the vfs setup - this code sets up ambient files and from DT, they have assumptions likeallowSyntheticDefaultImports being set due to the node ecosystem e.g. the discussion in denoland/deno#17058

you could maybe add a sigil and check for it after imports?

declare module "blah" {
  ... usual imports

  export const __jsrTest: Symbol
}

then run the type checker for code like

import {__jsrTest} from "blah"

but as you can imagine, there's a lot of trade-offs there

Comment on lines +165 to +169
`
declare module '${importSpecifier}' {
${importedCode}
}
`,
Copy link
Author

Choose a reason for hiding this comment

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

With fresh eyes I see the very obvious flaw here: The imported code has relative imports that won't exist in the context of this module. So anything defined and available in the actual code itself will appear fine, but anything imported from another file will fallback to being any.

So it does need to be a re-export of some sort but so far I've been unable to get that to work in most contexts.

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.

2 participants