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

File name deduplication not working with ParseAndCheckFileInProject #819

Closed
alfonsogarciacaro opened this issue Oct 4, 2017 · 9 comments

Comments

@alfonsogarciacaro
Copy link
Contributor

The new ParseAndCheckFileInProject is working great to get the AST of a single file, thanks again @vasily-kirichenko for that! I'm currently adding some dependency checks to Fable to make sure all the necessary files are being refreshed when a watch compilation is triggered. I'm having some issues, but hopefully I should be able to fix them soon.

However, I've also noticed the trick implemented by @forki to deduplicate file names #748 (so you can have two files in different folders with the same name in the same project) doesn't work if I try to get the AST of a file with the same name as a previous one using ParseAndCheckFileInProject (it works if I use ParseAndCheckProject)

Any idea what may be the reason?

@vasily-kirichenko
Copy link
Contributor

All tools (VS, Ionide, Rider) use ParseAndCheckFileInProject, yet the latter two works nicely on Fable projects containing files with same name in different folders. So my guess is that you call the function in a wrong way. First question: do you pass full file name to it along with content?

@dsyme
Copy link
Contributor

dsyme commented Oct 5, 2017

@alfonsogarciacaro I think I'd need to see the actual calls, e.g. by submitting a failing test. As @vasily-kirichenko says I suspect incorrect call parameters in some way

@alfonsogarciacaro
Copy link
Contributor Author

Sorry, I should have included instructions to reproduce the problem. You can do it with Fulma and Fable 1.3.0-beta:

git clone https://github.com/MangelMaxime/Fulma
cd Fulma
git checkout feature/test_fable_1_3
yarn
cd docs
dotnet restore
dontet fable webpack-dev-server

When Fable compilation is finished you can see the web in localhost:8080, then any change in any file will be automatically reflected. But if you edit the src/Fulma/Common.fs file you get the following error:

ERROR in ../src/Fulma/Common.fs
/Users/alfonsogarciacaronunez/dev/Fulma/src/Fulma/Common.fs(3,0): (75,0) error FSHARP: An implementation of the file or module 'Common' has already been given
 @ ./src/FulmaExtensions/PageLoader/View.fs 9:0-65
 @ ./src/FulmaExtensions/Dispatcher/View.fs
 @ ./src/App.fs
 @ ./docs.fsproj
 @ multi ../node_modules/webpack-dev-server/client?http://localhost:8080 webpack/hot/dev-server ./docs.fsproj

My suspicion is this happens because there's also another file named Common.fs in Fable.Elmish. @vasily-kirichenko here is the call to ParseAndCheckProject in Fable 1.3.0-beta, at that point msg.path should contain a full path.

Anyways, I'll try to submit a failing test 👍

@dsyme
Copy link
Contributor

dsyme commented Oct 5, 2017

@alfonsogarciacaro OK, I do see that that must be a problem - the modification implied by that de-duplication really needs to be applied to the ParseAndCheckFileInProject input too. Let me see if I can make a speculative PR which you can test

@dsyme
Copy link
Contributor

dsyme commented Oct 5, 2017

@alfonsogarciacaro So #820 should do the trick

@dsyme
Copy link
Contributor

dsyme commented Oct 5, 2017

@alfonsogarciacaro Tentative build of FCS 16.0.3 nupkg should eventually appear in artifacts of https://ci.appveyor.com/project/fsgit/fsharp-compiler-service/build/1586

@dsyme
Copy link
Contributor

dsyme commented Oct 5, 2017

@alfonsogarciacaro
Copy link
Contributor Author

Sorry for replying so late, @dsyme. I tried the nupkg and it's working, thank you very much! Would it be possible to publish the fix in Nuget?

@alfonsogarciacaro
Copy link
Contributor Author

I've temporarily published the tentative nupkg as Fable.FCS (I hope that's fine) and then published dotnet-fable 1.3.0-beta-002. @vasily-kirichenko Could you please give it a try? This version should recompile all files with a dependency to the modified file in a watch compilation. Please note you need also to update fable-loader to 1.1.4. Please note that I haven't updated other JS clients yet (Rollup, splitter), so they will ignore the dependencies. Pinging also @forki @MangelMaxime.

dsyme added a commit to dsyme/FSharp.Compiler.Service that referenced this issue Nov 3, 2017
… bug fsharp#819) (#3700)

* fix de-duplication of individual files

* bump fcs version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants