lrhn / dep-configured-imports

Dart Enhancement Proposal for Configurable Imports.
8 stars 1 forks source link

Support implementing cross-platform interfaces #2

Open nex3 opened 9 years ago

nex3 commented 9 years ago

This proposal doesn't support a use-case I think is important: implementing an interface from a platform-specific library in a way that works across platforms. In particular, if A is in dart:io, there's no way for my package to write B implements A in a way that is visible to code that runs in the browser.

Here's a more concrete example. Suppose I'm writing an HTML-sanitizing API and I want to take a user-supplied Element. Other than the Element itself, my API is perfectly platform-independent, and I want to make my package easy to use on the server as well as the browser.

String sanitizeHtml(??? element);

Meanwhile, unbeknownst to me, John has been hard at work on html5lib and has produced a full-featured server-side implementation of Element; for clarity, let's call this CrossPlatformElement. There's nothing I can write for the ??? above that will let me take both a native Element object on the browser and a CrossPlatformElement object on the server; even if John wants to have CrossPlatformElement implement Element, there's no way for to do so that's visible on the server.

lrhn commented 9 years ago

I don't see an easy way to make you able to "hot-plug" a different interface into a library that doesn't anticipate it. There is nothing you can write that makes your code able to accept, by name, a type that you didn't know about when you wrote the library.

What the library itself can do is to write "Element", and then import the Element interface that is available on the current platform. That does require you to know both implementations.

The other alternative is to go back and ask whoever wrote the first implementation to also define an abstract interface for their class, and expose that in a not-platform-dependent way. That's hard with dart:-libraries because they can't depend on packages, but for packages, you can either add a separate library with the interface, or a separate package, and make sure that the platform-dependent implementation implements the platform-independent interface.

munificent commented 9 years ago

What the library itself can do is to write "Element", and then import the Element interface that is available on the current platform.

One way to handle this is to limit platform-specific behavior to runtime implementation but make the static shape of all libraries always platform-independent. In other words, all "dart:" libraries are available on all platforms and have the same type signatures. It's just that some may not be usable at runtime on some platforms.

This was essentially Søren's proposal for handling this. It, in theory, means any Dart implementation needs to contain at least the shell of every "dart:" library. In practice, I think a deploy step and tree-shaking can eliminate almost all of that.

jmesserly commented 9 years ago

It, in theory, means any Dart implementation needs to contain at least the shell of every "dart:" library.

Right.

(Note that having "dart:html" import available on the VM doesn't imply cost. It means the default embedder can see "dart:html" and map it to some path on disk that has html.dart with interface definitions, so no snapshotting, etc. So you'd pay nothing if you don't use that file, and could even delete it for a really slimmed down VM+libs build)

Ideally, there'd be a way to map 'dart:html' on VM some way external to the embedder code. e.g. if we had https://github.com/lrhn/dep-pkgspec/issues/5, then you could define:

{
'package:foo': 'packages/foo'
'dart:html': 'packages/html5lib/html.dart'
}

Maybe a possible solution to this issue:

library html; // package:html
export dart.platform == "browser" ? "dart:html" : "package:html5lib/html.dart";

Now code can be platform agnostic by importing "package:html/html.dart" instead, and in fact, "dart:html" would not be necessary, except by this library. However, you'd want most Pub packages to switch over to the package:html version.

nex3 commented 9 years ago

I don't see an easy way to make you able to "hot-plug" a different interface into a library that doesn't anticipate it. There is nothing you can write that makes your code able to accept, by name, a type that you didn't know about when you wrote the library.

I'm talking about libraries that do anticipate being used across platforms. In my example, sanitizeHtml is written explicitly so that it can be used across platforms. As a package author, I want to make it as easy as possible for people to use on both the browser and the server. In order to be usable on the browser, it must be able to take a native Element; in order to be usable on the server, it must have some sort of reasonable type annotation.

Maybe a possible solution to this issue:

library html; // package:html
export dart.platform == "browser" ? "dart:html" : "package:html5lib/html.dart";

Now code can be platform agnostic by importing "package:html/html.dart" instead, and in fact, "dart:html" would not be necessary, except by this library. However, you'd want most Pub packages to switch over to the package:html version.

How does this package work when the interface of Element changes? Is there any check that the Element from dart:html and the Element from html5lib have the same signature? This also has the downside, as you note, of requiring standardization of the package ecosystem.

jmesserly commented 9 years ago

How does this package work when the interface of Element changes?

tools would complain about incorrect usage, as they do today. For example, if running on the server VM, conceptually it sees:

library html; // package:html
export "package:html5lib/html.dart"; // dart:html on browser

where html.dart might be:

class Element extends Node {
  var sourceSpan; // assume only html5lib has this member
  ...
}

and my code might be:

import 'package:html/html.dart';
main() {
  var doc = new DomParser().parseFromString('<foo><bar>baz', 'text/html');
  print(doc.querySelector('bar').sourceSpan); // OK: has a `sourceSpan`
}

If running in the browser, or compiling with dart2js it would instead see (conceptually):

library html; // package:html
export "dart:html"; // package:html5lib/html.dart on server

On Dartium, that same program would print NoSuchMethodError: method not found: 'sourceSpan' Compiled with dart2js, it would issue a warning about the missing member.

Anyway -- that's just hypothetical. I'm not sure it would be necessary or useful to have different APIs, and for package:html, I think we'd be fine having if there was a restriction that the APIs must be equal. But it should work, conceptually.

seaneagan commented 9 years ago

I can't tell if this is close to any of the proposals above, but what about the cross-platform library defining all its own interfaces and conditionally import platform specific impls:

library html; // package:html

import (src/server.dart or src/browser.dart conditionally);

abstract class Element {
  Element() = ElementImpl;
  //...
}

sanitizeHtml(Element el) ...
library html.src.server; // package:html/src/server.dart

// Server impl.
class ElementImpl {
  ElementImpl();
  //...
}
library html.src.browser; // package:html/src/browser.dart

// Browser impl.
class ElementImpl {
  ElementImpl();
  //...
}
nex3 commented 9 years ago

@jmesserly That only comes up when the runtime usage of the declaration doesn't work; what if I'm just analyzing the package:html library on its own, rather than code that uses it?

@seaneagan That doesn't seem substantially different from the original proposal.

jmesserly commented 9 years ago

@seaneagan funny enough, we actually have a feature much like that, but only available to the core libraries: external and patch files. @nex3 I think what Sean is getting at is the APIs are required to be the same.

library html; // package:html

patch (src/server.dart or src/browser.dart conditionally);

abstract class Element {
  external Element();
  //...
}

sanitizeHtml(Element el) ...

// in another file
// Server impl.
patch class Element {
  patch Element() // ...
}

// in another file
// Browser impl.
patch class Element {
  patch Element() // ...
}
jmesserly commented 9 years ago

@nex3 wrote:

That only comes up when the runtime usage of the declaration doesn't work; what if I'm just analyzing the package:html library on its own, rather than code that uses it?

Analyzer is like other tools, it needs to know the configuration of the project.

Indeed, it already does this for package:, which cannot be resolved from the Dart source alone. When I click go to definition on Element, which file I go to can change. One moment it takes me to line 2273 analyzer/src/generated/element.dart, and the next it takes me to analyzer/src/element.dart on some other line, because I ran pub upgrade and it turns out they refactored away the "generated" folder and added some classes. (that's just a hypothetical example ;) ... the "generated" folder is still there in latest analyzer package, AFAIK)

Other languages have solved this as well (classpaths, defines, debug/release configuration, etc), so it's definitely a surmountable challenge. And it's no issue for VM/dart2js.

tatumizer commented 9 years ago

Sorry I'm late to the show, not sure I understand the problem. Why do you want to specify exact type here? String sanitizeHtml(??? element); You can leave it as dynamic, and everything will be fine.

Granted, you certainly have to know SOMETHING about the element to be able to handle it. E,g. you may need an the element, whatever it might be, to provide methods setHtml, getHtml, and the problem you are trying to solve is: how can I let compiler know about my assumptions?

If this is correct, then I think the problem is similar to this one: you have a method that accepts parameter "weight in kilos". Not to be confused with pounds. How can you let compiler know about it? Maximum you can do is to name parameter "weightInKilos", but this won't stop people from passing pounds. Basically, if you want to address all problems like this at the level of compiler, it will turn into Ada, and still won't be enough.

(Maybe one day compilers will start treating parameter names in more meaningful manner, but this won't happen soon) My point is that you either should have signature String sanitizeHtml(anyObjectHavingHtmlField); // which is the equivalent of "weightInKilos",

or simply write in dartdoc that so and so, I expect set/getHtml.

nex3 commented 9 years ago

@jmesserly Having to tell the analyzer a single configuration to parse is a pretty serious drag, for the reasons discussed in #4. Also, that still doesn't give me any confidence that the two Elements are the same across configuratoin.

@tatumizer I want my API type-annotated for the same reasons that I want any API type annotated: it provides better documentation, better tooling, and better readability.

tatumizer commented 9 years ago

@nex3 I think it would be a good idea to file a separate proposal for something like duck typing then. It's a more general issue, only tangentially related to configuration.

mezoni commented 9 years ago

@jmesserly

And you will solve a lot of problems with the analyzer and (as a bonus) with the coordination of different implementations. Later step would be reconcile "configured imports" in view of these language innovations.

mezoni commented 9 years ago

@jmesserly

Sorry, I talk about these features

abstract class Element {
  external Element();
}

patch class Element {
  patch Element() {}
}
mezoni commented 9 years ago

No need to be afraid of this (dangerous) features.

abstract class Element {
  external Element();
}

patch class Element {
  patch Element() {}
}

In each languge exists some "all bets are off". unsafe (C# Reference) https://msdn.microsoft.com/ru-ru/library/chfa2zb8.aspx

unsafe static void SquarePtrParam(int* p)
{
   *p *= *p;
}

The game is worth the candle.

jmesserly commented 9 years ago

@jmesserly Having to tell the analyzer a single configuration to parse is a pretty serious drag, for the reasons discussed in #4.

Sure, but it's something C#/C++ IDEs have had to deal with forever. It's a solved problem.

Also, that still doesn't give me any confidence that the two Elements are the same across configuration.

It doesn't, but that could be useful. If it worries you, it's easy to write a tool to automatically check. We already have the problem that there's no confidence of having the same API between package versions. This would be a simpler check: it's equality instead of a harder to specify notion of "compatibility".

mezoni commented 9 years ago

@jmesserly

Sure, but it's something C#/C++ IDEs have had to deal with forever. It's a solved problem.

You forgotten to mention about that the it requires different manual configurations.

Project settings -> Configuration properties -> Preprocessor, and edit the preprocessor definitions manually.

Or maybe know automated testing tools that creates different definitions and run them one by one?

As always, the Dart developers solves their problems through the "undocumented" features such as "patch", "external" and cannot solve another problems gracefully.

The proposed "configured imports" does not configures anything. They are the same as the conditional directives but very confusing and with ugly syntax. If not exists problem with analyzer (as you say It's a solved problem) then why don't use the way with a solved problem? This way called the 'preprocessing' and conditional directives is a part of it.

If you want fully transparent and clean way to building and testing of the cross-platform libraries then an "undocumented" features such as "patch", "external" are very well fit into ways of solving this problem.