Update for Dart 1.0 release #731

MathieuLorber · 2013-11-20T19:41:50Z

A refresh for the release of Dart 1.0 !

selector api breaking changes
files are moved to a vanilla directory, as it's not a MVC implementation (as vanillajs) (as discussed here Dart breaking changes #711 )
use of a dedicated dependency for UUID
HTML escape is done by std lib
compiled file update
.maps file (for sourcemap) is commited
directories layer simplified (/web directory is not mandatory)
readme update

As a result of libs use, the final compiled file is now 200k instead of 130. Pruning is not completely ok yet i guess =]. Using librairies still seems to be the good practice to me...

Sorry for previous PR, I did not merged my repo properly =/

sindresorhus · 2013-11-20T20:20:41Z

vanilla-examples/vanilladart/dart/TodoApp.dart

@@ -43,7 +45,7 @@ class TodoApp {
 			if (e.keyCode == KeyCode.ENTER) {
 				var title = newTodoElement.value.trim();
 				if (title != '') {
-					addTodo(new Todo(UUID.createUuid(), title));
+					addTodo(new Todo(uuid.v4(), title));


since it's only used once:

addTodo(new Todo((new Uuid()).v4(), title));

It's used each time you add a task, and Uuid construction isn't free

Same thing for HTML escape, I'm even thinking it could be global.

Ok, HtmlEscape construction is free, so let's forget about it.

I have no clue what best practises in Dart are, but aren't there const and final modifiers that could be applied to this?

They could actually be both (uuid and htmlescape) static & final... I look at that

I pushed it.

BTW, a lot more fields could be final (all fields in TodoApp.dart actually)... I'm not a "final extremist" + not sure it worthwhile here... WDYT ?

MathieuLorber · 2013-11-21T00:26:02Z

I did not though to the link in todomvc.com . Do I forget another one ?

MathieuLorber · 2013-11-21T00:42:30Z

No more absolute path in sourcemap but still, relative paths to the dart sdk were relative to my environnement

addyosmani · 2013-11-26T11:51:28Z

@sethladd if you have any time this week to help us review the Dart 1.0 changes to the app that would be hugely appreciated. Thanks!

sethladd · 2013-11-27T17:37:15Z

Happy to help. Looks like I can check this out tomorrow if that's ok?

sindresorhus · 2013-11-27T18:09:36Z

sure

sethladd · 2013-11-29T17:15:11Z

vanilla-examples/vanilladart/dart/TodoApp.dart

@@ -1,6 +1,9 @@
 part of todomvc;

 class TodoApp {
+
+	static final uuid = new Uuid();


Use a type annotation here. Put them on static fields for better code completion and documentation.

sethladd · 2013-11-29T17:27:14Z

It would be really nice if the app follow pub conventions. That way we can run pub build to create a nice output directory.

Is it OK if we had both a web/ and a build/ checked into TodoMVC? web/ would be the source and build/ would be the output.

sindresorhus · 2013-11-30T23:41:40Z

Is it OK if we had both a web/ and a build/ checked into TodoMVC? web/ would be the source and build/ would be the output.

Yes

addyosmani · 2013-12-02T14:35:11Z

+1. Thanks for the review, @sethladd!

Also, here in Paris atm with @MathieuLorber :)

sethladd · 2013-12-05T00:03:32Z

@MathieuLorber could you create a web directory, move the HTML, Dart, etc files into it, and then run pub build, which generated a build/ dir, and then check all that in? :)

MathieuLorber · 2013-12-05T08:35:42Z

Yep I didn't forgot =]. Pretty busy last days, I work on your code review soon !

sethladd · 2013-12-05T17:23:40Z

No problem, thanks!

MathieuLorber · 2013-12-09T09:51:59Z

Ok so if I understand the result of pub build, index.html is supposed to be in /web and get copied to /build ? There is no standard way to have an unique index.html which opens dart or js depending on the browser ?

passy · 2013-12-09T10:01:55Z

That's also very relevant for the AngularDart submission.

sethladd · 2013-12-13T01:43:55Z

Why do we need base.js? That generates an AJAX request for getFile('learn.json', Learn); which is an error.

sethladd · 2013-12-13T01:59:39Z

See MathieuLorber#1 because I couldn't find a good way to comment on random bits of code (unless that code was part of this diff? How do I leave a comment on a non-diff code?)

passy · 2013-12-13T10:22:04Z

@sethladd base.js is part of the JavaScript we have in all TodoMVC apps. It shows the learning bar, but since that's in the root folder, you have to start your http server there.

sethladd · 2013-12-16T19:16:43Z

lgtm, can we merge?

Thanks! 🎉

passy · 2013-12-16T19:20:39Z

@sethladd Some of your comments aren't addressed in the current version, like missing type annotations for UUID.

MathieuLorber · 2013-12-16T19:21:59Z

Sorry I couldn't look at it during last days. I'm just wondering one thing (can't verify right now), does the compiled version works without doing pub install first ? I'm thinking about dart.js for example

passy · 2013-12-16T19:23:50Z

@MathieuLorber No worries!

I found two issues wrt spec compliance:

entries should always be trimmed when saved
pressing escape when editing an item should revert the edit

sethladd · 2013-12-16T19:29:14Z

Thanks @passy ! We can fix those.

@MathieuLorber it should work out of the box, as long as you open and run from build/

sethladd · 2013-12-16T20:12:00Z

Hitting ESC now clears out the input.

Entries are trimmed for me. That seemed to work before. I assume you mean entering " hello " gets trimmed to "hello" ?

sethladd · 2013-12-16T20:12:35Z

Apologies, this PR is not off the branch I was using. I think I need to send over a new PR.

sethladd · 2013-12-16T20:13:46Z

I think we should close this PR and move the conversation over to #748

MathieuLorber · 2013-12-17T00:08:18Z

Sorry @sethladd , actually you commited on a branch I created for tests, I wanted to merge it to the branch of this PR when all was ok... But no problem =)

Update for Dart 1.0 release

6d58363

sindresorhus reviewed Nov 20, 2013
View reviewed changes

MathieuLorber added 2 commits November 20, 2013 21:39

One key:value by line

1d4a943

todomvc.com link to dart sample update

7754662

Commiting sourcemap file is not a good idea actually

507455e

MathieuLorber added 2 commits November 26, 2013 13:12

UUID & HtmlEscape are now static & final

f276686

Reordering

17ce5bb

sethladd reviewed Nov 29, 2013
View reviewed changes

MathieuLorber mentioned this pull request Dec 8, 2013

Add AngularDart example without routing #741

Closed

passy closed this Dec 16, 2013

MathieuLorber mentioned this pull request Dec 17, 2013

Update for Dart 1.0 release (new PR) #748

Closed

MathieuLorber deleted the dart-v1-release-PR branch March 15, 2015 13:05

Update for Dart 1.0 release #731

Update for Dart 1.0 release #731

Conversation

MathieuLorber commented Nov 20, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MathieuLorber commented Nov 21, 2013

MathieuLorber commented Nov 21, 2013

addyosmani commented Nov 26, 2013

sethladd commented Nov 27, 2013

sindresorhus commented Nov 27, 2013

Choose a reason for hiding this comment

sethladd commented Nov 29, 2013

sindresorhus commented Nov 30, 2013

addyosmani commented Dec 2, 2013

sethladd commented Dec 5, 2013

MathieuLorber commented Dec 5, 2013

sethladd commented Dec 5, 2013

MathieuLorber commented Dec 9, 2013

passy commented Dec 9, 2013

sethladd commented Dec 13, 2013

sethladd commented Dec 13, 2013

passy commented Dec 13, 2013

sethladd commented Dec 16, 2013

passy commented Dec 16, 2013

MathieuLorber commented Dec 16, 2013

passy commented Dec 16, 2013

sethladd commented Dec 16, 2013

sethladd commented Dec 16, 2013

sethladd commented Dec 16, 2013

sethladd commented Dec 16, 2013

MathieuLorber commented Dec 17, 2013