support typescript #129
support typescript #129
Conversation
|
We can look at the imports part together after #120 is merged! |
|
Related #136 which moves imported local ES modules to an _import path, which means we can rewrite these files more easily during preview and build. |
src/markdown.ts
Outdated
| id, | ||
| root, | ||
| sourcePath, | ||
| inline: true, | ||
| sourceLine: context.startLine + context.currentLine | ||
| }); | ||
| // Detect that we need TypeScript. |
There was a problem hiding this comment.
Should we always do this (call transpileTypeScript)? There’s not really a downside, right? It should be instantaneous.
There was a problem hiding this comment.
can do, but I'm afraid that the code might change a little, making it slightly more difficult to debug (can we do source maps?)
There was a problem hiding this comment.
Hmm... if we want to do this in all cases for inline expressions, then maybe we should also do it always on js fenced code blocks, and dispense with the ts name? It would be a bit simpler to use, and not too magical.
There was a problem hiding this comment.
I’m happy to add this first, and do the transpilation for ES modules as a followup.
We should document that our TypeScript support doesn’t do any type validation, though (which significantly diminishes the value of TypeScript). Meaning, I would say that we don’t enforce types, not just that we “strip type annotations”. Maybe look at how esbuild describes it.
|
imports are working |
|
I've reworked this PR and it's now passing all the JS code (inline, fenced, and imports) through esbuild. |
There was a problem hiding this comment.
This is now up-to-date with main, and I’d say 98% ready in some sense. But I think there is a major problem with this approach, and also a small (but potentially challenging) issue with the implementation.
The major problem is that this “supports TypeScript” by stripping TypeScript annotations. Which means you don’t get any of the type safety of TypeScript. Which raises the question of why you’re using TypeScript in the first place? Is it just so the code runs without modification? Or do you want to have the added safety of TypeScript to validate the types? As an example, you can have a TypeScript code block like this:
const file: whatever = FileAttachment("file.txt");and it happily runs since the : whatever is stripped. My sense is that this isn’t what people want when they ask for TypeScript in Framework; I think they want type checking. And that means we can’t simply use esbuild to strip the type annotations — we’ll need to use tsc (or whatever) to check the types.
The minor problem is preserving the semantics of expressions. The current approach looks for a trailing semicolon, and doesn’t consider comments. I added a skipped test to demonstrate the incorrect behavior. I think to do this correctly, we probably need to parse the TypeScript and see if it’s an expression. Which means we need a TypeScript parser (not just Acorn). Which is likely more expensive than running esbuild.
I’m not really sure how to proceed here. But I think we should investigate type checking, probably using tsc, if we want to support TypeScript in Framework.
|
This is a first step that allows you to write ts (or copy/paste ts) and run the code. But the hope is that we'll be able to get type checking with a VSCode extension #882? 🤔 |
|
As a user I don't want tsc to run with the framework, or at least not in dev, I can do that separately before deploy, once. Storybook also ran tsc for the users and it made it very slow, they added a config flag to skip checks, which is a blessing, on by default now iirc. I think the value of TypeScript will be provided by the editor itself running the checks and autogenerated definitions to glue parts of the framework together (that can come later) Just being able to load ts files is plenty of gain. |
|
Now that I read it again, I want to clarify. tsc should be able to run and detect errors, just not constantly in dev, but as a stand alone command; would expect Not sure if thats what you mean it's not working? |
|
Yes we should consider landing this without enforcing type checking and just stripping type annotations. We’ll need to merge main but that should be doable. |
|
superseded by #1632 |
TODO:
tstag? if all thejscells are "cleaned up"…closes #79