fable-compiler / repl-legacy

http://fable.io/repl
MIT License
14 stars 10 forks source link

[WIP] Update to Fable 1.3 #7

Closed MangelMaxime closed 6 years ago

MangelMaxime commented 6 years ago

6

@ncave I am trying to update the repo to latest version of Fable.

I updated the makeProjOptions function to something similar to what I saw on Fable repo. And I deactivated the completion feature because I don't really what I should at it yet :)

But even, with Ionide being happy I am getting an error when building Build.FCS target.

fable: Compiled ../Fable/src/dotnet/Fable.Core/AST/AST.Babel.fs [!] Error: 'defaultArg' is not exported by ../Fable/build/fable-core/Util.js https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module src/Fable.REPL/fcs.fs (7:9) 5: import { FSharpParseFileResults } from "../../../FSharp.Compiler.Service_fable/src/fsharp/vs/ServiceUntypedParse.fs"; 6: import { FSharpProjectOptions, FSharpCheckProjectResults, FSharpCheckFileResults } from "../../../FSharp.Compiler.Service_fable/src/fsharp/vs/service.fs"; 7: import { defaultArg, comparePrimitives, equalsRecords } from "../../../Fable/build/fable-core/Util"; ^ 8: import { map } from "../../../Fable/build/fable-core/Array"; 9: import { SymbolLookupKind, getSymbol } from "./lexer.fs";

Can you please help me either by pointing me the direction or contributing back in this PR ?

alfonsogarciacaro commented 6 years ago

Weird, defaultArg has moved to Option.ts. Not sure why it's being called from Util.ts, are you using latest dotnet-fable?

MangelMaxime commented 6 years ago

Ah... I didn't update the deps via paket. Perhaps this is the problem here. Let me see.

MangelMaxime commented 6 years ago

Hey hey @alfonsogarciacaro thanks you :) ❤️

It's now working, for a basic code:

let t = 1

open Fable.Import

Browser.console.log "maxime"

Now, I need to take a look at the autocompletion support and also see what change made @ncave in the Fable repo for the repl. And see, I am missing anything.

Fist trough here, it feels more stable and quicker than before to do the compilation. Perhaps, this is because, I removed the completion not sure yet.

alfonsogarciacaro commented 6 years ago

The completion should consume some CPU for sure, we need some way to optimize it. Currently it's set to be triggered just once per second. What kind of problems you have? Before I got too many results but this should have been fixed using FSAC lexer, see https://github.com/fable-compiler/fable-repl/issues/3#issuecomment-329959591

MangelMaxime commented 6 years ago

The signature of GetDeclarationListInfo have changed so I get a compilation error. And I don't know how to fix it.

ncave commented 6 years ago

@MangelMaxime @alfonsogarciacaro Thanks guys, sorry I was stuck on paket dependencies that break the build on my windows machine. I can't build (or even restore) successfully unless I comment the <Import Project="..\..\.paket\Paket.Restore.targets" /> in Fable.REPL.fsproj.

@MangelMaxime Two minor adjustments if I can suggest:

MangelMaxime commented 6 years ago

@ncave I update the PR with your two suggestions :)

And.. what have you done to my computer, me too I get a problem with the build system now. Was working fine 1 hour ago. :) For info, I am under linux so probably not OS related.

Using your fix:

<Import Project="..\..\.paket\Paket.Restore.targets" /> in Fable.REPL.fsproj.

Works...

So we now have two thing to figure out:

ncave commented 6 years ago

@MangelMaxime And two other minor things:

MangelMaxime commented 6 years ago

@ncave I updated it to 2.0.3 but now, the different FAKE build goes crazy. One install 2.0.3, then another install 2.0.2, then 2.0.3, etc.

This is compiling at the end but we should update the dotnet version in every FAKE script ^^.

Node dependencies has been updated to latest version but I am still having issues with UglifyJS.

Also, if you need feel free to push change into this PR.

ncave commented 6 years ago

@MangelMaxime Unfortunately I'm getting a runtime error (missing resource) with 2.0.3 on FCS_export, so it may have to stay with 2.0.2 for now.

ncave commented 6 years ago

@MangelMaxime But I have updated the FCS_fable to 2.0.3 so that should help with the build issue (2.0.3->2.0.2->2.0.3).

MangelMaxime commented 6 years ago

Ok no problem @ncave

I installed globally the 2.0.3, so should avoid Fake helpers to download each SDK each time.

MangelMaxime commented 6 years ago

Found another thing, seems like when doing the new architecture of the repo I forget to copy/paste the fable-core folder generated using Fable repo.

So for a quick test, I copy/paste it manually and now I see this error:

SyntaxError: export declarations may only appear at top level of a module CurriedLambda.js:1 SyntaxError: import declarations may only appear at top level of a module String.js:1

Code for reproduction:

let helloWorld = sprintf "Hello, %s"

Fable.Import.Browser.console.log(helloWorld "Maxime")

@alfonsogarciacaro Are we missing a babel plugin or a transformation ?

MangelMaxime commented 6 years ago

Ah ok I think I know what's wrong. I didn't build the fable-core modules for AMD. Will test it today and see if it's better :)

MangelMaxime commented 6 years ago

So the problem was indeed due to a non AMD version fable-core. I fixed the build.fsx to generate a custom AMD version of fable-core and copy the files in the right place.

ncave commented 6 years ago

@MangelMaxime

The signature of GetDeclarationListInfo have changed so I get a compilation error. And I don't know how to fix it.

https://github.com/ncave/FSharp.Compiler.Service/blob/fable/fcs/fcs-fable/test/app.fs#L204-L206 (note the column # should really be 24 in this case, but that's specific to this test script, I'll fix it later).

MangelMaxime commented 6 years ago

@ncave Thanks you, I will take a look at it this evening. And good to know there is a test folder can be helpful in the future too :)

MangelMaxime commented 6 years ago

Last commit, re-activate the completion service.

Can one of you explain me what this function is doing?

I believe it's use to make the result more accurate and less numerous but not sure :)

MangelMaxime commented 6 years ago

@ncave Should we open an issue on paket repo about ? Just to test I try to use netstandard2.0 but result is the same.

C:\Projects\fable-repl\src\Fable.REPL\Fable.REPL.fsproj : error NU1100: Unable to resolve 'System.ValueTuple (>= 4.0.0)' for '.NETStandard,Version=v2.0'. [C:\Projects\fable-repl\Fable.REPL.sln] C:\Projects\fable-repl\src\Fable.REPL\Fable.REPL.fsproj : error NU1100: Unable to resolve 'FSharp.Core (>= 4.2.0)' for '.NETStandard,Version=v2.0'. [C:\Projects\fable-repl\Fable.REPL.sln]

Edit: Seems related to: https://github.com/fsprojects/Paket/issues/2894

ncave commented 6 years ago

@MangelMaxime Opening a separate issue is probably a good idea.

ncave commented 6 years ago

@MangelMaxime

Can one of you explain me what this function is doing?

Source: https://github.com/fsharp/FsAutoComplete/blob/master/src/FsAutoComplete.Core/Parser.fs#L197-L224

MangelMaxime commented 6 years ago

Thank you @ncave

MangelMaxime commented 6 years ago

For reference: https://github.com/fsprojects/Paket/issues/2908