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

feat: typescript support! #4178

Draft
wants to merge 24 commits into
base: master
Choose a base branch
from
Draft

feat: typescript support! #4178

wants to merge 24 commits into from

Conversation

wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Apr 26, 2024

Details

Very much still a work in progress; opening draft PR to facilitate discussion.

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.
  • 💔 Yes, it does introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.
  • 🔬 Yes, it does include an observable change.

GUS work item

@@ -84,6 +84,9 @@ export default function scriptTransform(
// an error when the generated code is over 500KB.
compact: false,
plugins,
// TODO [#0]: This isn't needed for JS. What happens in projects that don't include
// typescript as a dependency? May need to use separate JS + TS transformers.
presets: ['@babel/preset-typescript'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Our strategy in the past has been that consumers of LWC are responsible for providing something to transform TS into JS (e.g. a Rollup plugin). By the time the code reaches @lwc/compiler, it should already be JS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like babel doesn't need typescript as a peer dependency.

npm i @babel/cli @babel/core @babel/preset-typescript
npm ls typescript # not present

echo 'export const foo: string = 123' > index.ts
echo '{ "presets": ["@babel/preset-typescript"] }' > babel.config.json

npx babel index.ts -o compiled.js # works!
cat compiled.js # export const foo = 123

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our strategy in the past has been that consumers of LWC are responsible for providing something to transform TS into JS (e.g. a Rollup plugin). By the time the code reaches @lwc/compiler, it should already be JS.

In the past, we haven't had official support for TypeScript!

Copy link
Contributor

Choose a reason for hiding this comment

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

The similar change was rejected earlier here #3471 (comment). The reasoning is basically "the LWC compiler shouldn't be in charge of transforming the TypeScript source into JavaScript.".

Copy link
Member

@jmsjtu jmsjtu Apr 26, 2024

Choose a reason for hiding this comment

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

Just to add some more context, we're trying to see how to map the LWC compiler errors back to the source TS files.

For example, if a customer authors something like this in their JS today:

import { LightningElement } from 'lwc';

export default class extends LightningElement {
    @hello world
}

They'll get a red squiggly around @hello world that reads

LWC1102: Invalid 'hello' decorator usage. Supported decorators (api, wire, track) should be imported from "lwc"

This is done in the language server by running transformSync from the OSS compiler, grabbing the source location and emitting it in the vscode editor.

We're trying to see how we can map the errors back to the source file when it's a TS file instead of JS.

Having said that, I don't think we necessarily need to include this in the OSS compiler but maybe we can add it to the language server and pass it to the OSS compiler.

Also, I think if we were to support TS here we'll probably need to adjust @lwc/metadata to handle it too.

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.

None yet

4 participants