Nagareyama does not watch multiple fsproj (and multiple F# source directory) projects

tomcl commented 3 years ago

Description

nagareyama does not watch files

Repro code

Not sure if here is the best place for Fable 3 feedback? I have made progress with the above error but now I've got to a stage where i need watch compilation to investigate - otherwise it takes too long. electron-webpack does this fine, but fable 3 does not watch files in my project:

C:\github\issie>dotnet fable watch src/renderer
Fable: F# to JS compiler 3.0.0-nagareyama-rc-007
Thanks to the contributor! @Pauan
src\renderer> cmd /C dotnet restore Renderer.fsproj
  Determining projects to restore...
  All projects are up-to-date for restore.
Parsing src\renderer\Renderer.fsproj...
Initializing F# compiler...
Compiling src\renderer\Renderer.fsproj...
F# compilation finished in 9920ms
Skipping Fable compilation of up-to-date JS files
Compiled src\renderer\Renderer.fs
Fable compilation finished in 582ms
Watching src

The above compiles the code, and electron-webpack will HMR ok. But if I change src/renderer/renderer.fs (with a js output change) it does NOT get recompiled.

Expected and actual results

I expect the src/renderer directory to be watched for Fs file changes. it is not.

Related information

Fable version: 3.0.0-nagareyama-rc-007
I suspect the issue here may be because I have 3 linked F# projects in src/renderer, src/common, src/simulator. The message 'watching src' is reasonable, but what is needed is to watch the subdirs. (And, in fact subdirs of subdirs since I have source code divided into different subdirectories within one project).

src  ----> renderer ----> renderer.fsproj
                     ----> renderer.fs  (entry point, in renderer.fsproj)
      ----> common    ----> common.fsproj
      ----> simulator ----> simulator.fsproj

I have tried changing src/renderer.fs (does not recompile).

The system here is windows.

alfonsogarciacaro commented 3 years ago

Hi @tomcl! Yes, it's ok to report Fable 3 issues here and I appreciate you do it 😊

Hmm, I've tested Fable 3 in several projects containing project references and all source files (including subdirectories) are being watched. However, I think the latest release had a bug I just fixed where if the first compilation contained F# errors, changes didn't trigger a watch recompilation. Is that your case? If not, does this happen always (the watcher doesn't react to any change)? Have you tested with other projects? Is there something particular in your setup? Would it be possible to create a repo to reproduce the issue?

tomcl commented 3 years ago

Thanks! Ok, so my repo is very complex, I thought I would try on cmeeren's fable-elmish-electron-material-ui repo first. My code is loosely based on that.

That is an electron-webpack app. On it (single fsproj for renderer and main) watching works perfectly, as does fable watch -> electron-webpack dev bundling. I think. Except that:

  ERROR in ./src/Renderer/App.fs.js
  Module build failed (from ./node_modules/babel-loader/lib/index.js):
  SyntaxError: C:\github\fable-elmish-demo\src\Renderer\App.fs.js: Unexpected token, expected "," (202:4)

The .fs.js output seems not to be liked by electron-webpack

This is definitely a different error from the above. however I'd like to get past it to a simple working demo first before going back to my project. FYI - my project - unlike this simple demo - does completely compile and run

repo is https://github.com/tomcl/fable-elmish-demo/tree/dev-fable3 (note the dev-fable3 branch in the link).

Re your other possible for the original watch bug: the code had no errors. from my previous testing the watcher did not ever recompile anything. I suspect it as being something silly except that there is a lot of system and tool similarity between the fable-elmish-demo build (which watches fine) and my repo build (which does not).

My "not watching" repo is https://github.com/tomcl/ISSIE again dev-fable3 branch. But I don't guarantee this will be very smooth to build, the npm stuff needs sorting out (it does work, but the build file uses npm whereas I think it may need yarn to do initial install). I will sort this out if I can't replicate the bug on something simpler, so you might want not to try it yet.

tomcl commented 3 years ago

The Fable compilation output from App.fs:

export const Theme_example = (() => {
    let color, color_1, styles_1, arg10, styles_2, props, props_1;
    const props_2 = ofArray([["palette.type", "dark"], (color = blueGrey, ["palette.primary", color]), (color_1 = purple, ["palette.secondary", color_1]), (styles_1 = ofArray([mkStyle("fontWeight", "bold"), (arg10 = singleton(mkStyle("backgroundColor", "#7FFFD4")), (mkStyle("\u0026$disabled", createObj(arg10))))]), ["overrides.MuiButtonBase.root", createObj(styles_1)]), (styles_2 = ofArray([mkStyle("borderWidth", 10), mkStyle("borderColor", "#000000"), mkStyle("borderStyle", "solid")]), ["overrides.MuiAvatar.img", createObj(styles_2)]), (props = singleton(mkAttr("size", "small")), ["props.MuiButton", createObj(props)]), (props_1 = singleton(mkAttr("fullScreen", true)), ["props.MuiDialog", createObj(props_1)])]);
    const merge = [];
    let theme;
    theme = (function setProperty(target, key, value) {
    const sepIdx = key.indexOf('.')
    if (sepIdx === -1) {
    target[key] = value
    } else {
    const topKey = key.substring(0, sepIdx)
    const nestedKey = key.substring(sepIdx + 1)
    if (target[topKey] === undefined) {
    target[topKey] = {}
    }
    setProperty(target[topKey], nestedKey, value)
    }
    }
    const target = {}
    for (let kv of props_2) {
    setProperty(target, kv[0], kv[1])
    }
    return target
    );
    return StyleImports_createMuiTheme_argsArray(theme, ...merge);
})();

Clearly not as should be, coming from:

module Theme =

  // Not used, but shows how to use style and prop overrides. The returned theme
  // can for example be used as the `theme` prop of `Mui.muiThemeProvider`.
  let example = Styles.createMuiTheme([
    theme.palette.type'.dark
    theme.palette.primary Colors.blueGrey
    theme.palette.secondary Colors.purple

    // Globally override component styles
    theme.overrides.muiButtonBase.root [
      style.fontWeight.bold
      style.inner "&$disabled" [
        style.backgroundColor.aquaMarine
      ]
    ]
    theme.overrides.muiAvatar.img [
      style.borderWidth 10
      style.borderColor.black
      style.borderStyle.solid
    ]

    // Globally override component props
    theme.props.muiButton [
      button.size.small
    ]
    theme.props.muiDialog [
      dialog.fullScreen true
    ]

  ])

I'm not actually that concerned about this bug: it does not show in any of my code, I'm not sure if it merits its own issue.

tomcl commented 3 years ago

OK, I have a clean build for my main repo, showing the "no watch" bug. Other than that it compiles fine and starts - there seems some additional bug (maybe not fable3, but different from previous fable2 build). I will sort that out if possible after watch compilation works, since watch/HMR recompilation will make investigation a lot quicker. If the additional bug exists I'll flag it as a separate issue.

Clean build repo showing "no watch" bug:

https://github.com/tomcl/fable-elmish-demo/tree/dev-fable3

Note must be dev-fable3 branch.

build (windows) or dotnet fake build will install and build code. It will enter watch/hmr mode, but this does not work because fable3 watch does not watch.

npm run compile will show the watch bug, compiling main process (OK) and then compile/watching renderer process - which compiles ok but does not watch e.g. change src/renderer/renderer.fs

npm run watchmain shows that it does correctly watch the (much simpler structure) main project.

PS - I'm very much looking forward to using Fable3. it will make a much faster dev cycle, and also since the app uses maps a lot maybe speed up the app. I hope Nagareyama can be fairly stable by mid-Jan next year because I will then have 60 3rd year uni students all working on this code...

inosik commented 3 years ago

The incorrect code from your previous comment should be fixed with this: cmeeren/Feliz.MaterialUI#58. Can you please try the latest version of Feliz.MaterialUI? Note that this is a major update, from 0.13.1 in your repository to 1.2.2.

inosik commented 3 years ago

As far as I can tell, your build process (both in https://github.com/tomcl/fable-elmish-demo/tree/dev-fable3 and in https://github.com/tomcl/issie/tree/dev-fable3) simply doesn't invoke Fable before running Webpack. With Fable 3 we don't have a dedicated loader for Webpack anymore, so the Fable compiler needs to be executed first.

tomcl commented 3 years ago

I invoke Fable 3 as follows in my repo EDITED: https://github.com/tomcl/issie/tree/dev-fable3 "dev": "dotnet fable src/main && dotnet fable watch src/renderer --run electron-webpack dev". I expect this to compile main process (it does) and then compile (yes it does) and watch (no it does not I think this is a bug) the renderer process.

The default build calls this script.

I was mistaken in my above comment suggesting npm run compile which will never watch because it invokes fable without watch.

But in any case I was also testing with just dotnet fable watch src/renderer. That is the simplest way to see the no watch bug.

tomcl commented 3 years ago

OK - I've run femto on that the cmeeren repo and updated all dependencies, including material-ui. It does not change the bug which shows syntactically incorrect javascript coming out from syntactically correct F#. There may be some bad emit definitions in there somewhere I guess, but it is not obvious to me.

But personally, I have no concern about this issue, it is not my code, and my code base all compiles into correct javascript. (I think).

inosik commented 3 years ago

Hm, I can't seem to find the dotnet fable substring in the repository (tomcl/fable-elmish-demo). But dotnet fable watch src/Renderer/ --run electron-webpack dev works on my machine as well, until it runs into the invalid syntax in the generated JS file.

tomcl commented 3 years ago

Sorry - I posted the wrong link! I meant My repo https://github.com/tomcl/issie/tree/dev-fable3

The cmeeren repo does watch fine. So ignore this repo. I only highlighted it because there appeared to be a separate bug somone might want to know about.

I've now sorted out my repo (spent all last night doing this) to build easily and simply, shows the watch bug in a structure with 3 linked projects and multiple source directories.

fable watch src/renderer

Fabel3 says "watching 'src'" but does not watch the subdirs src/renderer etc.

tomcl commented 3 years ago

OK - so I now I think understand the watch problem. It is just a feature, not a bug, as follows.

Watch works, on all .fs files in the project, or dependent projects, providing the current directory is the same as the .fsproj directory.

Thus for a project src/renderer/myproj.fsproj:

dotnet fable watch src/renderer will compile but not watch

dotnet fable watch . --cwd src/renderer will compile but not watch.

cd src/renderer

followed by

fable watch . will compile and watch.

This is a bit counterintuitive, but easy to work round in simple cases.

In a complex case like my repo, where the dev script needs to run and watch main project, then run and watch renderer project, then run electron-webpack which itself runs and watches, it is more difficult. It would help if there was no requirement for current directory to be the same as the fsproj file.

This set of scripts works for my electron app. and typical compile and load time is 1/10 what it was before: previous solution would recompile all files.

    "dev": "cd src/main && dotnet fable watch . --run npm run devr",
    "devr": "cd src/renderer && dotnet fable watch . --run npm run webpack",
    "webpack": "electron-webpack dev",

alfonsogarciacaro commented 3 years ago

Thanks a lot for investigating this and for the repo sample @tomcl @inosik! I'm very pleased to hear that typical compilation is 10x faster than before :tada:

It should be possible to watch subdirectories even if you're not in the same directory as the project. I usually run dotnet fable from the repo root and the watcher works fine. If you're interested this is how the watcher is started:

Start a FileSystemWatcher with filter .fs(x|proj) that includes subdirectories
After Fable compilation is finished, select the common base directory for all source files, set it as the path of the watcher and enable raising events.
When an event is raised, check the file belongs to the project

https://github.com/fable-compiler/Fable/blob/41ccfc253c890292b2514402089c594eabf485cf/src/Fable.Cli/Main.fs#L206-L241

I've tested the dev-fable3 branch you kindly prepared in the issie repo. It was working for me but I had to adjust the command. Maybe Fable watcher was working but it didn't trigger a webpack compilation because --watch was missing when invoking Webpack. The following command worked for me from the repo root:

dotnet fable watch src/Renderer --run webpack --config webpack.additions.renderer.js --watch

Every time I changed Renderer.fs a new bundle was created. However the bundle was failing because the webpack.config.js was missing Sass and file loaders, as well as the electron-renderer target.

@tomcl Could you please try if that command ☝️ works for you? BTW, Issie looks awesome! I downloaded it and played a bit with it. But I only have an empty project so it's not very exciting to share. Do you have a more complicated project or some screenshots, gifs to tweet about it?

tomcl commented 3 years ago

Thanks! The thing is electron-webpack is a module that does all that stuff (webpack + setup + watching). So web-config.js is auto-generated and not normally used, and the "additions" files are minimal.

I was finding just running dotnet fable watch on its own that it was not recompiling when watching in the cases I stated - but maybe I was doing something silly wrong? Or, does fable3 expect to interface with webpack in some way, rather than just communicating via the watched files (it watches the .fs, generates js, and electron-webpack or webpack watches the .js)?

I have a fully working build now with the last three lines of my last post. It may be weird but it watches everything and auto-compiles and HMRs.

Yes, I'm very pleased with Issie. It has a quite wonderful UI for its purpose. Its main problem is draw2d which is an awesome library but v complex and the InteractiveManhattanRouter in it which allows very intuitive quick automatic routing of connections has a js bug that leads to infinite loops with stack overflow sometimes when moving components and therefore auto-routing connections. In addition, draw2d does a lot of v expensive computing of bounding boxes - quite unnecessary in our use case - that makes it very slow. It can take 15s to load a large design sheet and that is all Draw2d taking a long time to add figures to its canvas.

So it seems a good idea to make a fable/elmish cut-down equivalent for what Issie needs. That is planned for Spring to be integrated into Issie next Summer if all goes well.

We have some quite big Issie projects - biggest is a full CPU with about 10 design sheets. I'm holding off publicising it till the build and the features are a bit more sorted out. We will have heavy use by students with complex circuits next term, this Term is lighter use ironing out bugs, so it should be good for advertising then.

Porting to Fable 3 the two issues for me (apart from weirdness watching) were:

(1) SimpleJson.stringify - I had an older version of the package without serialize. The Readme well documents this must be used with fable 3. No problems changing it to Json.serialize<'T> once I updated packages. Maybe Fable3 could have min package version check or something? But I don't properly understand nuget/paket packaging.

(2) The app used List.distinct on a JS object list. This broke under Fable3 as you can imagine. I used distinctBy and a string id to mend this.

(3) I measured Map performance vs old Fable. My test case was maps of size 3, 20, 100. I looked at map lookup, and map update, as the two operations. For all of these speed was very similar. Fable 3 coming in about 10% better for the bigger maps.

Here is a (simple) circuit that shows it doing something and lets you run the wave simulator - which is pretty good.

alfonsogarciacaro commented 3 years ago

Thanks a lot @tomcl! I tweeted about Issie and people love it! https://twitter.com/FableCompiler/status/1329451503107641348?s=19

About SimpleJson, it relied on some implementation details that have changed in Fable 3, but Zaid have fixed and in Fable stable release we will indeed provide a way to distinguish the compiler version to support both Fable 2 and 3 if necessary. Also @inosik has fixed the issue with List.distinct!

reinux commented 3 years ago

I'm not sure if this is the same problem, but I don't seem to be able to get it to watch my folder calling dotnet fable watch either in the same directory as the project or with the project specified. No luck with dotnet fable watch . either. Running RC8.

Edit: No luck with RC10 either.

tomcl commented 3 years ago

What worked for me with a complicated electron setup that requires watching main and renderer processes:

    "compile": "dotnet fable src/main && dotnet fable src/Renderer",
    "dev": "cd src/Main && dotnet fable watch . --run npm run devrenderer",
    "devmain": "cd src/Main && dotnet fable watch . --run npm run webpackdev",
    "devrenderer": "cd src/Renderer && dotnet fable watch . --run npm run webpackdev",

In all cases watch is used on .. --run is used to run webpack while still watching. To watch both main and renderer --run is used twice.

inosik commented 3 years ago

Can you please check if dotnet watch -p src/Renderer test works for you? Just to check if the actual file watching works at all. The command will run dotnet test if any file in any referenced project changes.

tomcl commented 3 years ago

yes, dotnet watch -p src\Renderer seems to work fine both inside and across projects.

inosik commented 3 years ago

Maybe #2301 could help with this? Otherwise I'm running out of ideas. We'd either have to somehow debug this, or begin from scratch and add stuff until it begins to break.

reinux commented 3 years ago

I think I got it!

Whether it's System.IO.Path.GetFullPath or System.IO.Directory.GetCurrentDirectory(), Windows will only return an API caller the name of the folder as it's given in the parameter, regardless of whether or not it's the correct capitalization.

So System.IO.Path.GetFullPath @"c:\windows" will always return "c:\windows" even though it's technically "C:\Windows". Same thing applies if your working directory in a shell or build tool or what not is incorrectly capitalized.

As a result, when Fable.Cli.Main.FsWatcher.Observe compares the changed file path to the list of files to check, if the working directory or path in the command line are incorrectly capitalized, it won't match.

The simple solution is to call ToLower() (or ToUpper()) on fullPath and on each item in filesToWatch first. This will produce some false positives on case-sensitive file systems.

To be absolutely certain that it won't, the correct solution might be to use one of the solutions to this SO question -- though, honestly, it's probably safer to just go ahead and recompile any file that happens to have the same name with different capitalization than it would be to risk any false negatives caused by the SO solutions being untested on some file systems.

tomcl commented 3 years ago

If wanting to dot is and cross ts you could check the whole path for duplicates and issue a severe warning without compiling the wrong-cased stuff. Following the principle of strong typing you could argue that enforcing case discipline in this way would be better than working round lack of such discipline which might break other tools!

But I am more of a pragmatist, in this case I think working around case issues - and also issuing a warning, would be the optimal solution. FABLE solves a complex tech problem, and anything to reduce newbie issues is good,

This is clearly a case where I am guilty of severe disorganisation in not paying more attention to filename casing. It is however a pain, and reinux's suggestion would put this right.

alfonsogarciacaro commented 3 years ago

Oh my, thank you so much @reinux! That makes total sense. Actually, I'm an idiot because we had a similar problem when deduplicating the paths in the output directory and didn't occur to me that it could be the same with the watcher. I will apply your solution, false positives shouldn't be a big issue here and it's better to be safe than sorry as you say. Thanks again!

reinux commented 3 years ago

Cool, thanks @alfonsogarciacaro !

I just read a little more about case sensitivity in file systems because it's literally keeping me up at night, and it's actually kind of horrifying -- and not even just on Windows.

For example, if you mount a case-insensitive file system on *nix, which at its root is case sensitive, parts of the path will be case-sensitive, and others not. The notion of a "case-sensitive file system" is also kind of ambiguous, with NTFS itself apparently being case-insensitive whereas the Win32 APIs aren't -- until you turn on case sensitivity using fsutil.exe on a per-directory basis.

Soooo... I've probably made the same mistake 3269874 times as well, and I won't even know until I'm missing a file or something.

Up is down, black is white and the earth is flat 😐

tomcl commented 3 years ago

... and @reinux deserves more than one hooray emoticon!

Cleaning up casing solved my weird build bug #2285 #2293. But see there for comment, because I think there is an issue with libraries in .fable directories.

fable-compiler / Fable