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

(fable-compiler, REPL) Incremental building for projects with multiple files #1648

Closed
ncave opened this issue Nov 21, 2018 · 16 comments
Closed

Comments

@ncave
Copy link
Collaborator

ncave commented Nov 21, 2018

As we now have a use case for potentially editing multiple files in a future version of the REPL, it would be nice if we can do incremental builds so we don't have to recompile all files every time there is a change.

Continuing the discussion from here.

Add ParseAndCheckFileInProject in Fable REPL.

@ncave
Copy link
Collaborator Author

ncave commented Nov 21, 2018

A good first step towards it would be adding parse and typecheck caches (with hashed keys to reduce GC).

@alfonsogarciacaro
Copy link
Member

If we manage to get this the next (or parallel step) would be to add support for libraries and the question would be how to resolve the package references without resorting to MSBuild (or could we communicate with it?). I mention it because Fable 2.1 will be distributed through npm so it'd be interesting to add an option to fable-loader to switch the dotnet or js compiler. However, the libraries will still be distributed through Nuget. We already tried distributing F# libraries with npm in Fable 0.7 but it didn't play very well with F# tooling. Though we could entertain again the idea if it helps fable-compiler(-js), what do you think?

Maybe we could detect Fable packages in node_modules and automatically inject .dll references to the .fsproj if needed. This would also remove the need of having to move F# sources from the Nuget cache to .fable folder to prevent Webpack module resolution issues.

@ncave
Copy link
Collaborator Author

ncave commented Nov 21, 2018

@alfonsogarciacaro Currently the fable-compiler[-js] only handles project references, not package references. Technically if those binary dependencies can be resolved somehow (through dotnet restore, or somehow else), they can be added to the metadata list (after the .fsproj is cracked, before the parsing), the same way Fable does it, more or less. If we can reliably identify which packages were pre-compiled with Fable, maybe we can other things too. Perhaps an example repo that uses already [pre]compiled libraries will make it more clear what needs to be done, as it's still a bit unclear to me what the ultimate goal or vision is.

On a side-note, if you need to rename the fable-compiler package to fable-compiler-js, to make things less (or even more :) confusing, by all means do so.

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Nov 21, 2018

Thanks! Yes, probably renaming it's a good idea 👍 The mention about packages distributed through npm was not about precompiled libraries, sorry I was not clear, just the same way as we do now (sources + .dll). I'm reluctant to distribute precompiled libraries because there may be issues when using a different compiler version. Actually, it already happened in the repl fable-compiler/repl#80

I was thinking that it could be easier to resolve package dependencies if they're in npm. But you're right, we can just get the information from projects.asset.json if necessary. Forget what I said 😸

@ncave
Copy link
Collaborator Author

ncave commented Dec 11, 2018

@alfonsogarciacaro

  • Added parsing and type check caching to FCS-Fable's ParseAndCheckProject_simple (only).

I opted for simplicity and not changing the current interface. Functionality is the same, just recompile the whole project every time, if something can be reused, it will be. If you hold on to the checker, and if no source was changed, or only a few at the end, it will be much much faster the second time. There is also an explicit cache clean method, if memory pressure becomes an issue.

@ncave
Copy link
Collaborator Author

ncave commented Dec 11, 2018

@alfonsogarciacaro The next step will be to try the fable-compiler-js with hot reload :)

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Dec 11, 2018

Awesome! I guess the cache is invalidated with content hashes, right? Which means it'll work both on the node app and the browser repl.

@ncave
Copy link
Collaborator Author

ncave commented Dec 11, 2018

@alfonsogarciacaro Correct, fable-compiler-js and fable repl are the same thing, so yes.

@alfonsogarciacaro
Copy link
Member

I edited my comment above, but you were faster to reply ;) I'll implement this in fable-compiler-js 👍 Probably the easiest way to implement hot reload is to integrate it with fable-loader. I'll try to do it so we can start uniforming the dotnet and JS versions :)

@ncave
Copy link
Collaborator Author

ncave commented Dec 11, 2018

@alfonsogarciacaro I'm not sure what you mean by uniforming, currently fable-compiler-js is also doing the fable-splitter role (albeit the output folder structure is different (not flat), it's preserving the original folder structure, so IMO it's simpler and better :). What's still missing from fable-compiler-js apart from hot reloading is copying the javascript imports (dependencies) (if any) to output folder. Technically this is not part of compiling, can be done after.

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Dec 11, 2018

We're starting to have many flavors of Fable & friends, sometimes with duplicated behavior (as you just mentioned for fable-compiler-js and fable-splitter) which complicates maintenance. I was thinking that maybe we could forget about the dotnet/js division (becoming blurrier) and just have components with common protocols so it's easy to combine them. We could have these main categories:

  • Compiler: FCS/Fable (dotnet and js versions)
  • Libraries: Fable.Core, fable-library, fable-metadata, fable-repl-library?
  • Clients: fable-loader, fable-splitter, fable-repl... Get/watch the code and send it to the compiler, interact with Babel for the JS generation...
  • IDE: Monaco integration, React component

So in this view, fable-loader for example would interact with fable-compiler (dotnet) by default but users would have the option to use fable-compiler-js instead. Much like, the sass-loader can either use node-sass (c++) or (dart-)sass (js) (see the implementation option). In the future we could have a full JS scenario (fable-compiler-js with fable-loader and the repl IDE running as local server) or fable-compiler-js/fable-splitter using Visual Studio as IDE, etc.

It's work but it may save us from trouble in the future. What do you think?

@alfonsogarciacaro
Copy link
Member

BTW, I remember the flattened structure in fable-splitter was done to avoid problems with files coming from outside the project folder. Has this been solved now?

@ncave
Copy link
Collaborator Author

ncave commented Dec 11, 2018

@alfonsogarciacaro

I remember the flattened structure in fable-splitter was done to avoid problems with files coming from outside the project folder. Has this been solved now?

It was always a TODO: to come up with a better way of doing that. I think I have, and it's very minimal. It seems to work fine but it hasn't been tested on a large number of projects yet by the public. Technically it can be introduced as an alternative option in fable-splitter too.

The main reason fable-compiler-js is doing it's own splitting was to be lean and not require unnecessary dependencies (and also drive innovation without breaking things people depend on). But I share your point about the components, so to quickly get there, we can just add an AST message output option to fable-compiler-js, and that should cover it for now, since the main supported tooling is already built for it.

I'm not sure we can make the compiler tooling completely interchangeable with dotnet, as we're probably not going to re-implement all the features of MSBuild. There will be differences and corner cases and projects that need manual intervention to compile, but we can aim for a good percentage of the simpler F# projects, hopefully most of them are like that.

@alfonsogarciacaro
Copy link
Member

Published as fable-compiler-js 1.0.0-alpha-001. We probably need a test project so people can quickly give it a go.

@ncave
Copy link
Collaborator Author

ncave commented Dec 13, 2018

@alfonsogarciacaro Any F#-only Fable-oriented project should do.

@ncave
Copy link
Collaborator Author

ncave commented Dec 15, 2018

Closing as the stated goal is accomplished (see above), please reopen if needed.

@ncave ncave closed this as completed Dec 15, 2018
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

2 participants