Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

integrate with existing JS module systems #284

Closed
3 of 4 tasks
jmesserly opened this issue Aug 6, 2015 · 27 comments
Closed
3 of 4 tasks

integrate with existing JS module systems #284

jmesserly opened this issue Aug 6, 2015 · 27 comments

Comments

@jmesserly
Copy link
Contributor

jmesserly commented Aug 6, 2015

We have a few issues here:

  • linking: we shouldn't need to change the HTML file, as long as it references the entry point. We should be able to inject the <script> tags at runtime for everything else. (This pattern is fairly common, for example webcomponents.js)
  • bootstrapping: only the module system should be special. Everything else related to Dart support (SDK, DDC's runtime files) should be treated as normal code loaded through the module system.
  • use existing JS module system. Right now, we have dart_library, which is sort-of like AMD. But we should just use code/patterns that exists already.
  • Have some way of emitting different ES5 module patterns, because server and client are (usually) a bit different. More discussion of this in use harmony modules #34, about ES6 modules. If we could emit ES6 modules, Babel would offer users a way to transform to various ES5 formats, but this doesn't help us in the browser (we don't want another tool in the browser toolchain).
@justinfagnani
Copy link
Contributor

Regarding the browser, there are loaders like SystemJS that will convert ES6 modules to something else on the fly. The entrypoint output could set that up.

@Aetet
Copy link

Aetet commented Aug 13, 2015

I think it will be extremely useful to support such things as webpack-loaders. Which can help with incremental compilation and existing code integration.Is there any API to import existing es6 code from dart modules?

@jmesserly
Copy link
Contributor Author

Is there any API to import existing es6 code from dart modules?

yeah @jacob314 is working on something that will let you create a signature file for your JS code, then you can just call stuff in it.

@ochafik
Copy link
Contributor

ochafik commented Sep 8, 2015

Closure's goog.module looks promising (although doesn't support cyclic deps just yet), it seems to be the only option supported by the Closure Compiler, with plans to support ES6 modules and with node.js support.

I've started toying with it in the closure-modules branch but we'll probably need to write more of the core runtime in Dart first (this could easily be done with a pass-through JS interop).

@jmesserly
Copy link
Contributor Author

[...] we'll probably need to write more of the core runtime in Dart first

yep.

(this could easily be done with a pass-through JS interop).

hmmm, not sure we'd want "pass-through JS interop", as we're trying to move away from dart:js, which is only supported for backwards compatibility. We already have dart2js style JS builtin for inline JS, and we have @JsName for direct calls to JS objects that @jacob314 is working on. I feel like having yet another way of making JS calls will be confusing. One of the major goals of DDC is that when we're talking to a JS object, we can emit normal looking, "pass through" calls, without needing JsObject and stuff like that.

edit: formatting

@jmesserly
Copy link
Contributor Author

@Aetet had a good comment on this:

I think it sure be CommonJS, because it is wide spreaded enough. And transpiled es6 code with babel can import and export such code without any problem. Closure modules is not so good encapsulated as CJS and instruments for this python and java, which force you use another techs for just for build, where you can avoid them. So my vote is for CJS.

@jonaskello
Copy link
Contributor

I would vote for ES6 modules. They are the standard now so why support older module systems like CommonJS or AMD? From what I gather, if we could get standard ES6 modules as output we could easily make them work in the browser by either polyfilling the loading with SystemJS (as @justinfagnani pointed out) or just transform them with Webpack (as @Aetet pointed out).

@jmesserly
Copy link
Contributor Author

@jonaskello -- does any browser natively parse the ES6 module syntax? I don't think they do. So we can't generate raw ES6 modules and expect it to work in browsers.

I don't want to force our users to run yet another transform tool before their code will work. The whole point of dev_compiler is to have a fast edit+refresh loop.

Does node.js support ES6 modules? That would be a nice place to try them, if they work there. (I'm not sure.)

Believe me, the moment we can drop legacy ES5 module stuff, we will! But that day has not arrived yet.

@jonaskello
Copy link
Contributor

AFAIK nodejs support ES6 modules under a flag, but I haven't tried it:

node --v8-options | grep "in progress"
--harmony_modules (enable "harmony modules" (in progress))

Transforming ES6 to ES5 is already a well-solved problem in the JS world. Also hot-reloading is well-solved with webpack-loaders. Not sure of the value of solving them one more time in a Dart context. Although I do appreciate the effort to not introduce more tools. However my use-case is that I want to write libraries in Dart and consume in JS. With the output we get today that seems very difficult because I cannot plug that output into my JS toolchain (webpack, babel et al) because of the special module system used. I would rather have plain standard ES6 output, with standard ES6 modules (which is what I already use to write the JS application that would consume the Dart library) and just plug that output into my existing JS tool-chain.

Would it be possible to add an option to dev_compiler so it either outputs its own module system, or plain ES6 modules? That way it would be possible to have a choice of either one-tool solution, or plugging into an existing JS tool-chain.

@jonaskello
Copy link
Contributor

Thinking more about it, I guess there are two use-cases for dev_compiler. Either you use it to build an app in Dart and iterate in the browser using dartdevc --server. In this case the one-tool solution is good. The other use case, which I think dev_compiler is meant to solve, is to write in Dart and consume from JS. Is there an issue discussing this latter workflow or should I create a new issue for that? (Rather than pollute this issue further :-))

@jmesserly
Copy link
Contributor Author

@jonaskello, good points. A few thoughts:

  • yes we intend support both use cases
  • this issue is about supporting ES5 module systems
  • we intend to support ES6 modules as well, see use harmony modules #34. They will be the primary output format once more widely supported & we will phase out ES5.
  • we definitely aren't going to continue outputting a custom-system. The choices will be interoperable ES6, or interoperable ES5 :). Either way it will work with existing JS tools.
  • I continue to believe it's very important we handle this (quite straightforward) ES6->5 lowering in dev_compiler

@jonaskello
Copy link
Contributor

I wanted to try to build a webpack loader that would (re)compile dart files using dartdevc and then chain that to the babel-loader. The goal would be to support a fast edit+refresh loop in any browser. But currently blocked on that dev_compiler output can not be compiled by babel because of a confirmed bug in babel. Not sure that it would work anyway but could be an interesting experiment.

@vsmenon
Copy link
Contributor

vsmenon commented Nov 13, 2015

@jonaskello

You could try renaming the local timeout variable here:

[https://github.com/dart-lang/dev_compiler/blob/master/tool/input_sdk/lib/async/stream.dart#L1215]

and run:

./tools/build_sdk.sh

to avoid that particular case. But, it may occur elsewhere in the sdk. We try to preserve the original names whenever possible.

@jonaskello
Copy link
Contributor

I tried to run build_sdk.sh but I am not sure it is succeeding as it does not seem to update the files in lib/runtime/dart. I also get some severe errors in sdk_expected_errors.txt, not sure if that is related:

severe: [AnalyzerMessage] The redirected constructor '(dynamic) → JSArray<dynamic>' has incompatible parameters with '(dynamic) → JSArray<E>' (dart:_interceptors/js_array.dart, line 29, col 46)
severe: [AnalyzerMessage] Missing concrete implementation of 'num.==' (dart:_interceptors/js_number.dart, line 12, col 7)
severe: [AnalyzerMessage] Missing concrete implementation of 'String.==' (dart:_interceptors/js_string.dart, line 14, col 7)
severe: [AnalyzerMessage] The redirected constructor '((Stream<dynamic>, bool) → StreamSubscription<dynamic>) → _StreamSubscriptionTransformer<dynamic, dynamic>' has incompatible parameters with '((Stream<S>, bool) → StreamSubscription<T>) → StreamTransformer<S, T>' (dart:async/stream.dart, line 1571, col 9)
severe: [AnalyzerMessage] The redirected constructor '({handleData: (dynamic, EventSink<dynamic>) → void, handleError: (Object, StackTrace, EventSink<dynamic>) → void, handleDone: (EventSink<dynamic>) → void}) → _StreamHandlerTransformer<dynamic, dynamic>' has incompatible parameters with '({handleData: (S, EventSink<T>) → void, handleError: (Object, StackTrace, EventSink<T>) → void, handleDone: (EventSink<T>) → void}) → StreamTransformer<S, T>' (dart:async/stream.dart, line 1588, col 13)
severe: [AnalyzerMessage] The redirected constructor '((List<dynamic>) → void) → _SimpleCallbackSink<dynamic>' has incompatible parameters with '((List<T>) → void) → ChunkedConversionSink<T>' (dart:convert/chunked_conversion.dart, line 23, col 45)
severe: [AnalyzerMessage] The redirected constructor '(Iterable<E>) → LinkedHashSet<E>' has incompatible parameters with '(Iterable<dynamic>) → Set<E>' (dart:core/set.dart, line 75, col 41)

@jonaskello
Copy link
Contributor

Tried it some more times and now it actually worked. Must have gotten something wrong the first time around. Anyway, now I hit this error while compiling with babel. Not sure what to make of it, could be another babel bug?:

out/dev_compiler/runtime/dart/_classes.js -> out_es5/dev_compiler/runtime/dart/_classes.js
out/dev_compiler/runtime/dart/_errors.js -> out_es5/dev_compiler/runtime/dart/_errors.js
out/dev_compiler/runtime/dart/_foreign_helper.js -> out_es5/dev_compiler/runtime/dart/_foreign_helper.js
out/dev_compiler/runtime/dart/_generators.js -> out_es5/dev_compiler/runtime/dart/_generators.js
out/dev_compiler/runtime/dart/_interceptors.js -> out_es5/dev_compiler/runtime/dart/_interceptors.js
out/dev_compiler/runtime/dart/_internal.js -> out_es5/dev_compiler/runtime/dart/_internal.js
out/dev_compiler/runtime/dart/_isolate_helper.js -> out_es5/dev_compiler/runtime/dart/_isolate_helper.js
out/dev_compiler/runtime/dart/_js_embedded_names.js -> out_es5/dev_compiler/runtime/dart/_js_embedded_names.js
out/dev_compiler/runtime/dart/_js_helper.js -> out_es5/dev_compiler/runtime/dart/_js_helper.js
out/dev_compiler/runtime/dart/_js_mirrors.js -> out_es5/dev_compiler/runtime/dart/_js_mirrors.js
out/dev_compiler/runtime/dart/_js_primitives.js -> out_es5/dev_compiler/runtime/dart/_js_primitives.js
out/dev_compiler/runtime/dart/_native_typed_data.js -> out_es5/dev_compiler/runtime/dart/_native_typed_data.js
out/dev_compiler/runtime/dart/_operations.js -> out_es5/dev_compiler/runtime/dart/_operations.js
out/dev_compiler/runtime/dart/_rtti.js -> out_es5/dev_compiler/runtime/dart/_rtti.js
out/dev_compiler/runtime/dart/_runtime.js -> out_es5/dev_compiler/runtime/dart/_runtime.js
out/dev_compiler/runtime/dart/_types.js -> out_es5/dev_compiler/runtime/dart/_types.js
Error: out/dev_compiler/runtime/dart/async.js: We don't know what to do with this node type. We were previously a Statement but we can't fit in here?
    at NodePath.insertBefore (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/modification.js:62:13)
    at NodePath.unshiftContainer (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/modification.js:251:15)
    at Scope.push (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/scope/index.js:1031:41)
    at remap (/usr/local/lib/node_modules/babel-cli/node_modules/babel-core/lib/transformation/internal-plugins/shadow-functions.js:82:16)
    at PluginPass.ThisExpression (/usr/local/lib/node_modules/babel-cli/node_modules/babel-core/lib/transformation/internal-plugins/shadow-functions.js:22:7)
    at newFn (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/visitors.js:278:19)
    at NodePath._call (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/context.js:72:18)
    at NodePath.call (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/context.js:44:17)
    at NodePath.visit (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/context.js:102:12)
    at TraversalContext.visitQueue (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/context.js:151:16)

@jmesserly
Copy link
Contributor Author

Error: out/dev_compiler/runtime/dart/async.js: We don't know what to do with this node type. We were previously a Statement but we can't fit in here?

That sounds like an internal error in Babel of some sort. If you could figure out what code it was trying to compile I might be able to guess what the issue is.

@vsmenon wrote:

You could try renaming the local timeout variable here:

We could also just land a workaround for the SDK code, depending on how easy it is for Babel to fix.

@jonaskello
Copy link
Contributor

From the message it is not easy to know exactly what part of the code causes the babel error. If I get some spare time I could try to narrow it down somehow but meanwhile I created an issue over at the babel repo to see if they can help.

UPDATE: I went to check on the babel issue, but seems they have migrated the issues to another site. Now it can be found here. Still no action on it though...

@jonaskello
Copy link
Contributor

@jmesserly After a lot of investigation I was finally able to come up with a minimal repro. Trying to compile the code below with babel will give the same error (this is code cut down from the 6000 lines in async.js though manual binary search :-)). Any ideas what it might be that babel has trouble with? It seems related to the name "value" because if I change to "value2" in one of the two places the error goes away.

class Future {
  wait() {
    dcall(value);
  }
}

class _Future  {
  [_asyncComplete](value) {
    this[_zone].scheduleMicrotask(dart((() => {
      _chainCoreFuture(this);
    })));
  }
}

@jmesserly
Copy link
Contributor Author

Hmmm. That seems to work in the REPL. Any idea what version of Babel that is? I'll see if I can reproduce it locally

@jonaskello
Copy link
Contributor

Interesting, maybe it works with 5.x then (it says the REPL is using 5.x). I am using the latest version (6.1.18). Here is what I am seeing:

$ babel --version
6.1.18 (babel-core 6.1.19)
$ babel repro6.js --out-file repro_es5.js
Error: repro6.js: We don't know what to do with this node type. We were previously a Statement but we can't fit in here?
    at NodePath.insertBefore (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/modification.js:62:13)

@jmesserly
Copy link
Contributor Author

Seems to work:
babel --version

6.1.18 (babel-core 6.1.21)

babel lib/runtime/dart/async.js

... prints output JS code ...

@jmesserly
Copy link
Contributor Author

Ah, maybe that's it. Can you try upgrading to babel-core 6.1.21 and see if it fixes for you?

Not sure, but it looks like there were a bunch of recent fixes since .19: https://github.com/babel/babel/commits/master

@jonaskello
Copy link
Contributor

Upgraded but still same error (see below). However I am trying with my minimal repro file rather than the original async.js. I probably have a different async.js than you then, will check that.... If you try with the same content I have you probably get the same error?

$ babel --version
6.1.18 (babel-core 6.1.21)
$ cat repro6.js
class A {
  wait() {
    H(value);
  }
}

class B  {
  [C](value) {
    this[D].G(E((() => {
      F(this);
    })));
  }
}
$ babel repro6.js --out-file repro_es5.js
Error: repro6.js: We don't know what to do with this node type. We were previously a Statement but we can't fit in here?
    at NodePath.insertBefore (/usr/local/lib/node_modules/babel-cli/node_modules/babel-traverse/lib/path/modification.js:62:13)

@jmesserly
Copy link
Contributor Author

Your repro file works for me too, although it doesn't seem to transform anything. I guess you could try --no-babelrc. But the Babel folks might have some ideas, maybe ask on their issue tracker?

@jonaskello
Copy link
Contributor

Actually, in babel 6 it does not do anything by default (which babel 5 did). Which is probably why you are seeing the same output as input :-).

See this page:

https://babeljs.io/docs/setup/#babel_cli

Specifically step 4 near the bottom of the page which states:

Great! You've configured Babel but you haven't made it actually do anything. Create a .babelrc config in your project root and enable some plugins.

Note
Pre-6.x, Babel enabled certain transformations by default. However, Babel 6.x does not ship with any transformations enabled. You need to explicitly tell it what transformations to run. The simplest way to do this is by using a preset, such as the ES2015 Preset.

@jonaskello
Copy link
Contributor

Seems you can also specify "presets" (which is what transforms it will make) on the command line if you don't want to create a .babelrc file. So if you run this we will probably get the same output:

$ babel dev_compiler/lib/runtime/dart/async.js --presets es2015
Error: dev_compiler/lib/runtime/dart/async.js: We don't know what to do with this node type. We were previously a Statement but we can't fit in here?

@jmesserly
Copy link
Contributor Author

I filed #626 for requirejs support, so closing this one out.

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

No branches or pull requests

6 participants