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

Upgrade Fable #1415

Merged
merged 13 commits into from
Sep 20, 2020
Merged

Upgrade Fable #1415

merged 13 commits into from
Sep 20, 2020

Conversation

Booksbaum
Copy link
Contributor

@Booksbaum Booksbaum commented Sep 13, 2020

Fixes #1036 (though a couple of versions higher)

There are a couple of changes in the current fable version compared to the one used in current ionide -- and some are quite subtle. For example passed functions that previously were called now don't (ionide/ionide-vscode-helpers@ada99c8) or stricter handling for different types in F# (in code) and JS (during runtime) (92ecbd0).

I'm not sure I caught all those strange cases. The ones I found had strong impact -- like no files in Solution Explorer. But there might be other cases not so visible. And I didn't test it with a bigger project and over a longer period of time.
-> others should probably test it a bit more with their workflow

Requires ionide/ionide-vscode-helpers#33 and MangelMaxime/html-to-elmish#22 (-> reason this PR is marked as WIP)
For now the files for both are included from the PRs branches. So this should build just fine for testing.
I will change it back once the PRs are merged (and updated nuget package is released in case of html-to-elmish / Fable.HtmlConverter).

Update dotnet tools
Note: The files from `ionide-vscode-helper` are currently from
local folder `../ionide-vscode-helper` (`fable-upgrade` branch).
Useful for development (vs production): dev output (release/fsharp.fs)
isn't minimized.
Fable now throws an exception when `dict.Add(key,value)` for an
already existing value. Previously that worked.
Error was: Project files (and References and Items (`DTO.fs` ->
`Project`)) are stored as `list` in F# -- but during runtime (in JS)
it's actually an `array`.

In older Fable versions `List` functions worked with arrays. But now
calling a `List` function on an array returns an empty list.
```fsharp
// ls: (more than 0 values)
// * in F#: 'a list
// * in JS: 'a array
ls
|> List.iter (printfn "%A")
```
Previously: Prints each item
Now: Doesn't print anything

Note: Being an `array` is already the case in current ionide version
`4.16.0` and wasn't introduced by upgrading Fable. This might indicate a
mistake during (de)serialization, but I didn't look into it. Instead
`list` is just changed to `array`.
`Fable.HtmlConverter` `2.0.0` doesn't work with current fable
-> for dev: include fixed source from local folder `../html-to-elmish/`
Note: both are not served from the original repositories, but forked
repos with updated files. Will be reverted after the source repos were
updated.
Add framework restriction
Disable packages folder
@Krzysztof-Cieslak
Copy link
Member

Krzysztof-Cieslak commented Sep 16, 2020

Can you target 5.0 branch with this PR.

And wow, thanks a lot for those changes. Updating Fable is something we've put off for way too long

@Booksbaum Booksbaum changed the base branch from master to 5.0 September 16, 2020 11:24
@Booksbaum Booksbaum changed the title [WIP] Upgrade Fable Upgrade Fable Sep 19, 2020
@Booksbaum
Copy link
Contributor Author

ionide-vscode-helpers are from the ionide repo again. And Fable.HtmlConverter is updated and included as nuget package.
-> good to go

@cartermp
Copy link
Contributor

Some of the refactoring done (e.g., aliasing Func<obj, obj>) feels a little churn-y but otherwise these changes appear reasonable

@Krzysztof-Cieslak Krzysztof-Cieslak merged commit b1b29b8 into ionide:5.0 Sep 20, 2020
@Krzysztof-Cieslak
Copy link
Member

Well, this is awesome, thanks a lot ❤️

@Krzysztof-Cieslak Krzysztof-Cieslak mentioned this pull request Sep 20, 2020
8 tasks
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.

Upgrade to Fable 2
3 participants