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

[Master feature] TypeScript in AMP #13791

Closed
dreamofabear opened this issue Mar 3, 2018 · 22 comments
Closed

[Master feature] TypeScript in AMP #13791

dreamofabear opened this issue Mar 3, 2018 · 22 comments
Assignees
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request WG: infra
Milestone

Comments

@dreamofabear
Copy link

dreamofabear commented Mar 3, 2018

Add TypeScript support to AMP and allow new extensions to be written in TypeScript.

Motivation

Given the increasing maturity and popularity of TypeScript and potential developer productivity benefits, we'd like to experiment with TypeScript support in AMP.

Requirements

  • Supports existing toolchain, particularly Closure Compiler usage
  • Supports piecemeal adoption (with a rollback plan in case this I2I is rejected)

Proposal

  1. Experiment: Allow new extensions to be written in TypeScript, which will be transpiled to Closure-annotated JS via tsickle as a preprocess to our normal compilation pipeline.
  2. Gather data: Ship at least one new AMP extension written in TypeScript and collate pros/cons of development experience. Survey AMP community on TypeScript vs. JavaScript preferences.
  3. Convert: If gathered data is supportive, add support for incremental transpilation for remaining source and convert existing JS to TS file-by-file.
@dreamofabear dreamofabear added INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code WG: infra labels Mar 3, 2018
@dreamofabear dreamofabear self-assigned this Mar 3, 2018
@dreamofabear
Copy link
Author

@cramforce
Copy link
Member

I wish I had done this July 22nd 2015 :)

@dreamofabear
Copy link
Author

dreamofabear commented Apr 3, 2018

Everyone, please fill out the public preliminary interest survey! 🙏

@dreamofabear dreamofabear changed the title TypeScript in AMP [Master feature] TypeScript in AMP Apr 5, 2018
@dreamofabear
Copy link
Author

dreamofabear commented Apr 6, 2018

screen shot 2018-04-06 at 5 50 16 pm

Overall:
~15% disapprove, ~20% undecided and over 60% approve.

Positive sentiment:
Static typing should yield better productivity and less buggy code.

Negative sentiment:
Concerns about TS's longevity, learning curve, and opportunity cost of code migration.

See full survey results.

@ampprojectbot
Copy link
Member

This issue seems to be in Pending Triage for awhile. @danielrozenberg Please triage this to an appropriate milestone.

@achrinza
Copy link

Just checking in; is there any update on this? Currently this is causing friction in adopting Next.js with Typescript.

@dkolba
Copy link

dkolba commented Feb 26, 2020

@achrinza use this meanwhile

@dreamofabear
Copy link
Author

This experiment hasn't been prioritized yet. Deferring to @ampproject/wg-infra for scheduling.

@rcebulko
Copy link
Contributor

I'd be interested in taking a stab at Closure/Typescript interactions, what the migration process could look like, etc. Infra team has discussed using build-system as a jumping off point for the migration, since it doesn't reach the release and so would have less risk of causing a regression during migration, but would allow us to set up the main infra around building, running, and testing TS+JS

@fx69005
Copy link

fx69005 commented Jun 7, 2020

someone have news for this feature ?

@dreamofabear
Copy link
Author

There's insufficient staffing for this for the foreseeable future. Closing.

@SiaExplains
Copy link

Isn't there any news about the amp on ts still?

@samouri
Copy link
Member

samouri commented Aug 18, 2020

@SiaQnbr: no news beyond what you see here

@markhughes
Copy link

@choumx that does not seem like a decent reason for closure of a ticket, especially for something that could be automated on publish w/ something like https://github.com/urish/typewiz to at least provide something to devs.

@rcebulko
Copy link
Contributor

rcebulko commented Aug 21, 2020

@markhughes It seems like you have some familiarity with the relevant tooling here, so if you have an idea for a useful and non-intrusive way to address this, feel free to put a PR together and add myself/@ampproject/wg-infra to take a look at it. There's some added complication with migrating to TypeScript because of the various compilation and optimization steps that take place, which makes a transition of runtime code a bit risky, though we've had discussions about migrating some build-system code at some point. The work involved poses a not insignificant opportunity cost for the team to work on it, which I believe is part of why this feature request was closed.

@markhughes
Copy link

My comment was not about migrating to typescript on the spot. That was about adding typescript types.

For example: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/auth0

This would allow developers using TypeScript to use the package without the package actually migrating to typescript.

@samouri
Copy link
Member

samouri commented Aug 25, 2020

@markhughes: Can you explain more? In general, developers using AMP to build websites don't directly consume AMP JS APIs. They use our Web Components instead, e.g.

<amp-list> ... </amp-list>

I don't think TypeScript does html type-checking

@yassinebridi
Copy link

@samouri
Copy link
Member

samouri commented May 3, 2021

Turns out DevTools did something very similar in 2020. They also had a Closure Compiler-based JS repo and migrated to TS. They also wrote a post-hoc blog post.

@rileyajones
Copy link
Contributor

rileyajones commented May 25, 2021

I did some exploration into using TypeScript to type check the src directory to get a picture of the scope of the undertaking using this tsconfig.json
image

The compiler options are identical to those used in build-system and could probably be tuned to reduce the number of errors outputted, but the initial count I was presented with was 4331. While this number is large, it does seem surmountable.

This is the output of the command.
src_tsc.txt

@rcebulko
Copy link
Contributor

rcebulko commented May 26, 2021

@rileyajones See #34096 and #32693 for related work.
Have you found any benefits to TS over Closure typechecking?

@samouri
Copy link
Member

samouri commented Jan 14, 2022

Just realized this issue was already closed...but in fact, the FR is now complete as we are free to begin authoring TypeScript files.

See #37370 as the first example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request WG: infra
Projects
None yet
Development

No branches or pull requests