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

Update to TypeScript 5 (& dotnet 7 & Fable 4) #471

Merged
merged 16 commits into from Mar 26, 2023

Conversation

Booksbaum
Copy link
Contributor

@Booksbaum Booksbaum commented Mar 23, 2023

Updating to dotnet 7 & Fable 4 was trivial (mostly just worked).

Main change was moving the Fake build script into its own project (-> ./build/fake/). Necessary because dotnet fake doesn't work with dotnet 7. In the process I renamed fake.cmd/sh to build.cmd/sh. I think that fits better and should be more clear.
Changes when calling build script:

  • via script in ts2fable root: ./build.cmd RunTest
    • prev: ./fake.cmd -t RunTest
  • via dotnet: dotnet run --project ./build/fake/ -- -t RunTest
    • prev: dotnet fake build -t RunTest





Typescript 5 required some more changes:

To translate typescript.d.ts into valid TypeScript.fs:

  • Several added or adjust handlings for different stuff mostly done in previous PRs.
  • Some handling unique to typescript.d.ts (-> bottom of ./src/transform.fs). Thats guarded by a conditional compilation directive for TYPESCRIPT symbol (#if TYPESCRIPT) (-> special TS rules are not part of normal build!).
    • Pass --typescript-special-rules to build script to compile with special typescript rules (-> .\build.cmd BuildCli --typescript-special-rules).
  • Note: When upgrading TypeScript.fs: don't update TS directly in ts2fable, but use TS from another project.
    Reason: unbundled ts2fable CLI tool uses TS from node_modules -- which might not work when already upgraded (Though probably only an issue for major TS upgrades, not minor ones)
    (-> see first commit message in this PR)
  • typescript.d.ts to TypeScript.fs is done with ts2fable options --noresizearray --convertpropfns --remove-obsolete, but NOT --tagged-union: prev/current ts2fable doesn't use tagged unions -- and I thought it would be easier to upgrade without changing everything to tagged unions.

Some changes in new TypeScript:

  • Modifiers aren't part of Node any more, but instead limited to locations modifiers are possible. For ts2fable that means Accessibility and IsStatic is sometimes None/false where we previously processed modifiers. I think these are all locations Access/Static isn't possible in TS/F# (like Function Type) -- but it might also possible the modifiers just moved into a parent.
  • TS does now more JSDoc parsing. Which results in one regression (from a ts2fable point-of-view):
    • {@link SomeType|SomeTitle}: TS doesn't recognized | as separator -- but instead removes it, uses SomeTypeSomeTitle as link and no title....
      (Space as separator still works)

There are most likely more differences -- but at least the test all pass :)






I have also taken the liberty to increase the version number (to 0.9.0)


Note: Just documentation of commands to upgrade TypeScript. Actual upgrade isn't happening yet!

!!! upgrade typescript in another solution! not directly inside ts2fable!:
ts2fable uses ts from `node_modules` during runtime! -> Requires old (currently used) version, NOT new version

```powershell
dotnet fake build -t BuildCli

npm run ts2fable -- --noresizearray --convertpropfns --remove-obsolete "path/to/typescript.d.ts" "./src/TypeScript.fs"
```

Note:
* `--noresizearray`: Array instead of `ResizeArray`: usually don't need to add elements, but just consume -> array should be easier to handle
* `--remove-obsolete`: We throw away old functions. Might result in additional breakage, but should lead to simpler & cleaner upgrade path
* no `--tagged-union`: `member kind: SyntaxKind` instead of Tagged Union for `Node` Unions. Though not as nice in F#, probably easier to use for now because old `TypeScript.fs` doesn't use Tagged Unions.

Note: Result contains errors -> doesn't compile yet!

Note:
Upgrade directly to `5 beta` instead of stable `4.x`!

-----------------

powershell script I use for testing:
(requires prev ts2fable CLI build (-> `dotnet fake build -t BuildCli`))

(inside `typescript-test` with installed `ts 5 beta` in `node_modules`. ts2fable is in `../ts2fable`)
```powershell
$root = $PSScriptRoot
$dtsPath = "./node_modules\typescript\lib\typescript.d.ts"
$fsPath = "./src/TypeScript.fs"

Push-Location (Join-Path $root "../ts2fable")
try {
  npm run ts2fable -- --noresizearray --convertpropfns --remove-obsolete (Join-Path $root $dtsPath) (Join-Path $root $fsPath)
}
finally {
  Pop-Location
}
```
Some special rules are necessary to fix cases that aren't/cannot be handled with general rules
(Usually because Union (`Ux<..>`) in F# doesn't match some common base type constraint, while in TS that's possible)


-> generated `TypeScript.fs` compiles without error!

Note: Just `TypeScript.fs` compiles. ts2fable with new `TypeScript.fs` still produces compiler errors!
* building: pass `--typescript-special-rules` to Fake: `dotnet fake build -t BuildCli --typescript-special-rules`
  * Displays Warning at start & end of Fake output if enabled: `!!! Using TypeScript Special Rules !!!`
* code in ts2fable: hidden behind `TYPESCRIPT` constant -> if not defined: special rules don't get compiled at all

Note:
Test `tsc` is deliberately defined as `itOnly`: Always compile without TS special rules UNLESS you focus on those rules.
That has following ramifications:
* `--typescript-special-rules` with target `RunTest` will fail (because `itOnly`) -> use target `WatchAndRunTest` instead
* typescript special rules never get tested on CI!
TODO: Always the case? -> make common rule? `export =` is outdated and shouldn't be used -- but is used (including in `typescript.d.ts`...)
Note:
`Fable 4` is required for `dotnet 7` -> in same commit

Move build script into own project

Fake runner (`dotnet fake ...`) doesn't work with `dotnet 7`.
Fake build script is now a normal dotnet/F# project (`./build/fake`).
To run: `dotnet run --project .\build\fake\ -- -t TARGET` or `build.cmd TARGET`

Move `fake.cmd/sh` to `build.cmd/sh`
Update (some) F# code for TS 5
Notable changes:
* `string | null` is now `UnionType [StringKeyword, LiteralType (NullKeyword)]` (prev: `UnionType [StringKeyword, NullKeyword]` -- `string | undefined` is still `UnionType [StringKeyword, UndefinedKeyword]`)
  * TODO: check if that impacts transforms somewhere else
* JSDoc now now not just a string text, but also `JSDocText, JSDocLink, JSDocLinkCode, JSDocLinkPlain`
  * Note & TODO: not yet adjusted to new JSDoc Types -> just handled to compile, but not to produce correct output (-> comment tests still fail)
* `modifiers` isn't available on `Node` any more -> on some types no modifiers (Accessibility, static) any more
  * TODO: correct no Accessibility any more? Or must be retrieved from other element?
* `CompilerHost` uses now default `CompilerHost` from TS with some adjustments
  * which looks quite ugly because dynamic typing (because no setter any more for lots of functions)

Note: There are still tests failing -- but most tests pass :)

Note: to run tests (memo because changed in last commit):
`./build.cmd RunTest`
accidentally commented out in prev commit...
`describe` in mocha

`describeOnly` to focus on a single test suite
Regression because of how TSC parses links:
`{@link MyTarget|MyTitle}` isn't handled correctly, but instead parsed as target=`MyTargetMyTitle` and no title -- and bar disappears.
Using space as separator works.
Handling that would require some extended special handling -- which I think isn't worth.
There's an TS issue (which I'm too lazy to look up...), so it might get fixed in TSC in the future.
Note: `ts.createCompilerHost` uses local file system (o.s.) -- which does work for `ts2fable` command line tool, but not in web app
* Note: `CompilerHost` implementation doesn't use `{ new CompilerHost ... }`: some methods are optional in TS -- but required in F# (like `createHash`)
  -> to not specify these I'm using anon record type and force-cast that into `CompilerHost`
  TODO: How to express optional member in F#? setter with option? default implementation? (Nope: not available in F#, just C#)

TODO: Web-App produces warning:
> WARNING in ./node_modules/typescript/lib/typescript.js 3064:94-115
> Module not found: Error: Can't resolve 'perf_hooks' in '[...]\ts2fable\node_modules\typescript\lib'

`perf_hooks` seems to be part of node -> not available in web app

Web-App itself seems to work just fine
@alfonsogarciacaro
Copy link
Member

Fable 4 should be compatible with dotnet 7. Are you still having issues?

@Booksbaum
Copy link
Contributor Author

Fable 4 should be compatible with dotnet 7. Are you still having issues?

I meant Fake.

Fable 4 works perfectly with dotnet 7 👍

@alfonsogarciacaro
Copy link
Member

Ah, ok. Thanks for confirming!

@Booksbaum Booksbaum merged commit 23637f4 into fable-compiler:master Mar 26, 2023
@Booksbaum Booksbaum deleted the upgrade-typescript branch March 26, 2023 09:10
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

2 participants