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

TypeScript-related updates #2231

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Conversation

ncave
Copy link
Collaborator

@ncave ncave commented Oct 25, 2020

Various TypeScript-related improvements:

  • Fixed Map and WeakMap interface in Fable.Core.
  • Use Option instead of Nullable nodes in FSharpMap and FSharpSet (more idiomatic, little or no change in JS).
  • FSharpMap, FSharpSet, Dictionary and HashSet now implement ES6 Map and Set interfaces directly.
  • Changed sequence type from Iterable to IterableIterator for better integration with Map and Set interfaces.
  • Constructor functions now retain the actual baseExpr type.
  • Fixed extends printing for generic types.
  • Fixed naming for compiler generated idents.
  • Fixed paths with ${outDir} in them.
  • Fixed and regenerated Replacement injects.
  • Changed array creation to only use injected constructors.
  • Added Reflection info generic types.
  • Minor Types cleanup.

@alfonsogarciacaro
Copy link
Member

Thanks a lot! There seems to be movement in the ts2fable repo too so hopefully we can improve the (bidirectional) Typescript/Fable interop. BTW, would it be possible to write a short blog post explaining the Typescript feature from the user's perspective (how it works, when it makes sense to use it)?

@alfonsogarciacaro alfonsogarciacaro merged commit f21f810 into fable-compiler:nagareyama Oct 26, 2020
@ncave
Copy link
Collaborator Author

ncave commented Oct 26, 2020

@alfonsogarciacaro I'm not sure there is much to explain, it adds types when the --typescript flag is turned on, that's all there is.
I wouldn't presume to know whether it is of any use, there are still issues to iron out, so it's a work in progress. Perhaps enabling it in the REPL with an experimental toggle can give people an opportunity to try it and give feedback.

@ncave ncave deleted the next1 branch October 26, 2020 04:25
@alfonsogarciacaro
Copy link
Member

For example, when trying the feature I've noticed it needs the fable-library files compiled as Typescript. Should we add them to the dotnet fable package so they get injected into the .fable folder automatically when enabling --typescript?

About the REPL, it's a bit complicated. Should we?

  • Push a fable.io/repl3 version as is, without support for libraries like Fable.React or Fulma? (.dll only bindings like Fable.Browser should work)
  • Try to make the same hacks we did for the libraries in the current REPL? It may be a bit difficult because I already removed some of the code 😅
  • Wait until we can serialize the inlined functions from packages, so adding libraries to the REPL is easier?

@ncave
Copy link
Collaborator Author

ncave commented Oct 26, 2020

@alfonsogarciacaro As far as progress goes, we were able to compile fable-library to strict TypeScript in Fable 2.
In Fable 3, we had a bit of regression because of the many changes, but it's getting close again.
After that, the next big goal would be to compile all tests to strict TypeScript, but that has a much bigger scope.

You can try it to see where we're at by building the fable-library to TypeScript:

cd Fable
npm i
npm test
cd src/fable-standalone/test/bench-compiler
npm run build-lib-dotnet-ts

the output goes to the out-lib folder.

  • I never gave it much thought, but perhaps we package both TS and JS versions of the fable-library, and let the compiler copy the correct one to the target based on the flag.
  • For the REPL, I think the flag should only affect the code generation, and not run the generated code.

@alfonsogarciacaro
Copy link
Member

Awesome, thanks a lot for the info! Ok, I will try to push a Fable 3 repl and add Typescript output as option so users can start playing with it. BTW, if it's not too much to ask, would it be possible to rebase/merge your service_slim branch with dotnet/fsharp#9510 when you have time? I gave it a try, there were almost no conflicts but there were several problems during the build.

@ncave
Copy link
Collaborator Author

ncave commented Oct 26, 2020

@alfonsogarciacaro I'll take a look, there are quite a few changes and files moving around in dotnet/fsharp, just in the last few weeks since the last rebase, and apparently some assembly resolution changes, so the tests projects are broken. I may have to drop the use of Dotnet.ProjInfo for scripts, the same way Fable did.

@alfonsogarciacaro
Copy link
Member

Thanks @ncave! If it's becoming too hard to maintain the service_slim branch (besides fcs-fable) we could try going back to the standard FCS build/package. Or in order to keep the benefits of the service_slim for incremental compilations, if we need access to the FCS internals we could ask the F# team to expose what's need maybe in a internal module that we can more easily reuse in our custom builds.

Sorry, it's been a while since I haven't checked the service_slim code so I'm not sure if these proposals make any sense 😅

@ncave
Copy link
Collaborator Author

ncave commented Oct 27, 2020

@alfonsogarciacaro service_slim is usually not much of a problem, unlike Fable-FCS, which may get quite challenging after a lot of code churn. It's ok though, I enjoy digging through it and putting back the puzzle after it's been shaken really well.

@ncave
Copy link
Collaborator Author

ncave commented Oct 27, 2020

@alfonsogarciacaro I've rebased the service_slim branch to latest, and created another new branch for testing withessess, with latest + feature/fcsw called service_slim_fcsw:

Same instructions as before:

git clone https://github.com/ncave/fsharp.git
cd fsharp
git checkout service_slim_fcsw
bash fcs/build.sh

Binaries will be in artifacts/bin/FSharp.Compiler.Service/Release/netstandard2.0/

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