Fable icon indicating copy to clipboard operation
Fable copied to clipboard

[WIP] Quotation support

Open alfonsogarciacaro opened this issue 6 years ago • 23 comments
trafficstars

Continuation of #1818. Credit goes to @krauthaufen. Opening the PR to check the diffing with master while we evaluate. The branch has been published as fable-compiler-quotations to npm, can be tested by installing the package and then adding "compiler" to your Fable options like:

You also need to define the FABLE_QUOTATIONS constant to activate quotation support.

            {
                test: /\.fs(x|proj)?$/,
                use: {
                    loader: "fable-loader",
                    options: {
                        babel: CONFIG.babel,
                        define: "[FABLE_QUOTATIONS"],
                        compiler: "fable-compiler-quotations",
                    }
                },
            },

alfonsogarciacaro avatar May 20 '19 09:05 alfonsogarciacaro

With the fable-compiler-quotations, I get an error when using Elmish.Bridge.Client:

image

https://github.com/Nhowka/Elmish.Bridge/blob/91229e99cae0ebd923e22e8133ed6ed5fe60e605/src/Client/Library.fs#L43-L48

This is probably because it now includes more reflection info.

0x53A avatar May 28 '19 14:05 0x53A

Hey @0x53A you're right. It fails since it tries to find a constructor for the specific attribute. I'll take a look as soon as I have the time.. Cheers

krauthaufen avatar May 28 '19 17:05 krauthaufen

@0x53A i just committed a fix/workaround for the problem caused by unknown attribute types (which are simply omitted). The whole thing is a bit hacky and maybe @alfonsogarciacaro has a better idea...

krauthaufen avatar May 31 '19 21:05 krauthaufen

For this particular case it should be ok as that attribute just hides the method from intellisense, but as the feature enhances reflection it could enable some cool things with attribute support.

Nhowka avatar May 31 '19 22:05 Nhowka

it basically supports attributes on properties/methods/etc. but the attributes need to be defined in user-code, since they need to be constructed at runtime. I actually use field attributes in fshade extensively.

krauthaufen avatar Jun 01 '19 06:06 krauthaufen

since there seems to be some interest in quotations, here's a little status report:

  • "direct" (<@@>) quotations basically work
  • methods/properties/etc. used in quotations get resolved via new reflection things
  • when a quotation uses an undefined method (e.g. (+)) the runtime creates a MethodInfo and stores it in the (possibly new) declaring type. That way quotations can also use things which are normally replaced in Fable. Be aware that these MethodInfos cannot be called using Invoke. Currently Properties/etc. are simply resolved and the compiler just fails when using undefined properties in quotations (I'll fix that eventually)
  • ReflectedDefinitions are not created atm. but I intend to pass them (where needed) to the MethodInfo/ConstructorInfo creations.
  • Attributes are currently only supported on members, not on types atm.

krauthaufen avatar Jun 01 '19 07:06 krauthaufen

Yeah, attributes are not supported by current Fable. I was thinking that at some point we should add the possibility to get user-defined attribute info at runtime, but I'd rather not make all of them available. Maybe create a placeholder Fable.Core.RuntimeAttribute that users need to inherit to signal Fable this attribute should be available in runtime?

alfonsogarciacaro avatar Jun 03 '19 11:06 alfonsogarciacaro

Hey, the quotations branch currently tries to compile all attributes and (very hacky) recovers when something goes wrong. I totally agree that white-listing attributes via a special base-type would be a more elegant solution. I'll look into that. One problem however is that arguments for attribute constructions are currently limited to strings/numbers. However types should also be supported, since dotnet allows it. Cheers

krauthaufen avatar Jun 04 '19 06:06 krauthaufen

Hi, I know very little of quotations but was just wondering the following: Would System.Linq.Expressions be available with these changes? I've stumbled upon https://github.com/JamesRandall/AccidentalFish.FSharp.Validation and it would be great if I re-use the same validation rules on the client and server.

However it uses Linq and Fable cannot compile this atm, so I'm wondering if that would change with the quotations support.

nojaf avatar Aug 19 '19 09:08 nojaf

AFAIK Link expressions and F# quotations are two different things (not even sure if F# can generate Linq expressions from code as C# does) so I don't think that namespace would become available with this PR.

alfonsogarciacaro avatar Aug 19 '19 10:08 alfonsogarciacaro

Would love to see this finally land.

et1975 avatar Nov 02 '19 15:11 et1975

Yes, my last thinking was to turn this into a next branch and start releasing Fable 3 alpha versions with it. However, I've just started a new professional project and have very little time available for open source now, so I'm afraid I won't be able to meet the time and energy releasing a new major version requires.

It'd be great if someone would step up and lead the next major release. I would still be providing support, of course. Any volunteers? :)

alfonsogarciacaro avatar Nov 05 '19 21:11 alfonsogarciacaro

Hey, I'm not really capable of doing that, but I can certainly finalize the quotation/reflection things. I'm currently working on Fable.Elmish.Adaptive but once this is done I'll come back to this. Just let me know if I can help.

krauthaufen avatar Nov 05 '19 21:11 krauthaufen

gitpod-io[bot] avatar Apr 27 '20 06:04 gitpod-io[bot]

@alfonsogarciacaro See #2075.

ncave avatar Jun 04 '20 17:06 ncave

Hey there, I've been quite busy with other projects recently, but if there's interest in bringing that up to speed I'd totally be willing to help. Just let me know if you're still interested in integrating that...

krauthaufen avatar Jun 08 '20 06:06 krauthaufen

That'd be great @krauthaufen! I'm about to write an issue for the roadmap for Fable 3, so we can continue the discussion there. One quick question though, all the reflection support added in this PR is necessary for supporting the quotations or just to be able to execute a quotation in runtime?

alfonsogarciacaro avatar Jun 09 '20 06:06 alfonsogarciacaro

Hey, we could certainly support quotations without MethodInfo/PropertyInfo invokes but in many cases these are required... Maybe introduce a compiler-flag or something?

I'm not sure regarding the new reflection, but for quotations reflection needs to be more or less feature-complete since they need basically everything (ConstructorInfo, MethodInfo/etc.)

krauthaufen avatar Jun 09 '20 07:06 krauthaufen

It would be great if there was a way to tree-shake type data that isn't used. Then this becomes a non-issue

Shmew avatar Jun 09 '20 19:06 Shmew

Hello, I have a project that needs code quotations. What is the status of this? Is there anything I could do to help?

One idea I'm investigating is to create a separate Fable.Reflection NuGet that uses a plugin to add reflection data. Would that be a better approach if people are worried about adding extra data?

chkn avatar Mar 21 '21 16:03 chkn

I was planning to merge this with Fable 3, but then focused on other goals and unfortunately this branch is diverging more and more from the main one (nagareyama atm). Having a specialized Fable.Reflection package is a good idea, although I'm not sure if it can work with quotations, because these rely on the .net base reflection API if I'm not mistaken.

The first step would be to sync this with nagareyama again. My main worry is that iirc this branch increases reflection support but adds a direct reference to all class methods in the reflection info which will prevent tree shaking and increase the bundle size. A solution could be to only add methods to reflection info when a compiler constant is declared as it's already done with FABLE_QUOTATIONS.

alfonsogarciacaro avatar Mar 24 '21 15:03 alfonsogarciacaro

Having a specialized Fable.Reflection package is a good idea, although I'm not sure if it can work with quotations, because these rely on the .net base reflection API if I'm not mistaken.

The idea was to have a Fable.Reflection package implement the base .NET reflection API, and quotation support could be included in that or in another package that depends on it. Of course, having it "in the box" would also be good.. I'm just trying to understand the concerns.

My main worry is that iirc this branch increases reflection support but adds a direct reference to all class methods in the reflection info which will prevent tree shaking and increase the bundle size.

Hmm maybe we should differentiate the level of reflection support needed for code quotations from full support. Full reflection by its nature is not really amenable to tree shaking because someone could reflect any member of any type dynamically, so we must include all data. Code quotations on the other hand I think are fairly static expressions, where we know at compile time what reflection data is needed. In that case, perhaps they could just reference the specific reflection data and the rest would be omitted?

chkn avatar Apr 08 '21 14:04 chkn

Sorry for the late reply. I might be entirely wrong because it's been a long time since I haven't checked this branch, but I think the PR includes reflection support for invoking methods in order to run the quotations. Maybe it could be a good compromise to implement the PR without this for when there's no need to run the quotations, although unfortunately I'm not sure how much work that would entail.

alfonsogarciacaro avatar Apr 16 '21 15:04 alfonsogarciacaro