Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

Dynamic on JS side #925

Closed
wants to merge 3 commits into from
Closed

Dynamic on JS side #925

wants to merge 3 commits into from

Conversation

iskiselev
Copy link
Member

With this PR I moved processing of dynamic to JS side. We need this, as currently JSIL doesn't understand Roslyn dynamic patterns. Also, it will open path for full dynamic support (probably only with translated BCL).

This PR brakes support of Verbatium.Expression inside dynamic calls (VerbatimDynamic test moved to FailingTests). I will work on restoring it some day (if I'll here any complains - it will be sooner).

As using dynamic to work with JS objects provide not-very-readable and slow code, we need work on #532 ASAP as a primary way for JS interop.

This PR partially fix #913.

ILSpy version is updated to latest one with JSIL-specific patches. Would be good if somebody verify Mac build, as some patches was omitted.

Use JS-side callsite/binders analogs.

Updated ILSpy version.
CallSiteVariablesEliminated test deleted.
VerbatimDynamic test moved to FailingTests class.
@iskiselev
Copy link
Member Author

Let's wait for integration build results. Hope it should be good.

@kg
Copy link
Member

kg commented Dec 5, 2015

One alternative to #532 would be to do something like how packed arrays work, where the compiler is given extra type information and can use it to generate the raw variation on dynamic it produces right now. Not sure how viable that would be, though.

@iskiselev
Copy link
Member Author

I thought about additional type info for dynamics, but we will still need decompile dynamic calls back once again. We will need implement proper roslyn support for it and still have feature that works correctly only with C#.

As I already said somewhere, I see very small difference to:
SomeJSObject.Property1.Property2.SomeCall() and SomeJSObject["Property1"]["Property2"]["SomeCall"].Invoke() or even SomeJSObject["Property1.Property2.SomeCall"].Invoke()

@kg
Copy link
Member

kg commented Dec 5, 2015

The main problem with the non-dynamic form is that now you need to write dramatically different code to interact with JS objects. For example, now if you swap an OpenGL wrapper out for raw WebGL contexts, you have to rewrite everything.

@iskiselev
Copy link
Member Author

Interesting point. But it can be solved by using interfaces. You just need cover each used JS object with corresponding C# wrapper.
Even better, we may introduce some MetaAttribute, that will force dynamic-like behavior for some interfaces (you already created similar one for methods JSRuntimeDispatch, probably we just need extend it to interfaces).

@iskiselev
Copy link
Member Author

I even thoughts about automatically generation C# wrappers (and probably corresponding proxies) based on *.d.ts files.

@iskiselev
Copy link
Member Author

At the end, dynamic still works with raw JS objects, probably not so optimal and with less readable code.

@kg
Copy link
Member

kg commented Dec 5, 2015

Yeah, a combination of still-working-but-less-clean support for JS objects & dynamic, along with interfaces being able to point at raw JS objects will probably be good enough. Generating code from .d.ts would be cool.

@iskiselev
Copy link
Member Author

Looks that test were passed. Do you have any review notes or need some more time?

@kg
Copy link
Member

kg commented Dec 5, 2015

I will try to review it this weekend. I may merge it to a branch.

@iskiselev iskiselev force-pushed the Dynamic branch 3 times, most recently from bbfe241 to 7c20f6d Compare December 9, 2015 00:46
@iskiselev
Copy link
Member Author

Added new JsAPI. Found some problems, so we could discuss principal decisions, but PR is not yet ready to merge.

@kg
Copy link
Member

kg commented Dec 9, 2015

I dislike this approach to JS interop strongly enough that I think maybe something entirely different is necessary, like how Mono does inline assembly (which is literally impossible to find documentation for...)

For example, a more sophisticated take on Verbatim where you hand it a block of code and variables you want it to close over. We could even accept typescript in those blocks and have JSILc run those snippets through tsc with appropriate type information and shout at you if they fail to compile.

I think trying to enforce the .NET type system onto external JS objects is a bad idea and the extremely verbose code it produces demonstrates why. If we really want to produce raw JS, and we can't do it with dynamic, we should just write raw JS. It's not as if any JsAPI code will run on native .NET so the only advantage we get here is static type checking - but the static types aren't guaranteed to match runtime types anyway.

@iskiselev
Copy link
Member Author

Looks like we keep in mind different problems. This JsAPI doesn't try to give you way to create raw JavaScript code in C#. It will be much better to just write it in .js file, or, if you really need it - use Verbatium.Expression and write any code that you want.

On other hand, I'd like some wrappers, that will allow me to interact with JsObjects that was passed in my code with some filling, that it is still an object. Most of this work will only read/write one property or call one method.

Probably we should discuss it more. I hoped, that Adapt<T> will help me, but I don't see how it should work if some JsMethod expect to get some JsClass (so, object with some prototype).

@kg
Copy link
Member

kg commented Dec 9, 2015

On other hand, I'd like some wrappers, that will allow me to interact with JsObjects that was passed in my code with some filling, that it is still an object. Most of this work will only read/write one property or call one method.

Yeah, that's an important use case. But I can't see JsAPI as a worthwhile solution to that, it's too complicated.

@kg
Copy link
Member

kg commented Dec 9, 2015

I should clarify though: I think the idea of creating an adapter between a raw JS object and a .NET interface is a good idea, and that should be pursued for sure. It addresses many of those use cases.

Attempting to fully pull arbitrary JS objects into a statically typed universe is risky. Other than typescript I don't think anyone has done it successfully. Dart semi-famously has attempted to do this and failed spectacularly.

@iskiselev
Copy link
Member Author

When you say "complicated", do you mean that you should write many explicit code with casts?
As I tried preserve API as simple as possible. It contains only (Get/Set, Call, Invoke, New, Delete, In, AssumeType/As/Is/Cast). All other methods are just overloads for methods in list. Even in this list, As/Is/Cast are helpers that could be removed.

@iskiselev
Copy link
Member Author

Really, I started working on JsAPI, as after removing special dynamic handling, I was not very happy with Verbatium.Expression return dynamic - dynamic will spread through our source a little bit uncontrollable.

I'll fully OK, if for now, our main method of JS-interop will be Verbatium.Expression (and remove all code for #532), if it will return object instead of dynamic (same as other JS-interop methods). In most cases, in our project, returned object will be just saved as field or passed into next Verbatium.Expression.

@iskiselev
Copy link
Member Author

Maybe I'm incorrect, but I suppose that casting to dynamic should be optional - so user may cast Verbatium.Expression result to dynamic if he really needs it.

@kg
Copy link
Member

kg commented Dec 9, 2015

Yeah, I'm OK with stripping out dynamic. It breaks that 'dynamic DOM' example but not in an unfixable way. For things like WebGL I think we should just make interfaces work good.

TBH the codegen for dynamic has never been as good as I wanted anyway.

And yeah, they can cast to dynamic if they want and hit the (fully compatible, right?) binder-based slow path.

@iskiselev
Copy link
Member Author

I hope that with cast to dynamic everything will work (except passing to dynamic call Verbatium.Expression call #548), at least test are passed.

Let's confirm our plan:

  1. For now I will remove from this PR all work for JsObject API #532 and just change all JS-iterop API to return object instead.
  2. We may (and probably even need) continue discussion for JsObject API #532, but original thread will be probably better place.

@kg
Copy link
Member

kg commented Dec 9, 2015

Yeah. I'm OK with JsAPI living on in a branch - maybe we can find a good home/use case for it in the codebase - but for now I think we just want to change all the dynamics back into object. Alongside that we can think about how to improve the UX. I can imagine a way to make Verbatim work a bit better based on closures so that values can round-trip through reliably and evaluation order issues go away.

@iskiselev
Copy link
Member Author

Removed JsAPI - related commits into separate PR (#929).
This PR now could be treated as stable. If it would be merged, we need to reopen #548.

Should we use this or separate PR to change all the dynamics back into object?

@kg
Copy link
Member

kg commented Dec 9, 2015

Should we use this or separate PR to change all the dynamics back into object?

Separate PR, please. It'll probably require test changes and changes to the examples.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect FieldType resolution for closed generic
2 participants