Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Js_of_ocaml TodoMVC Example. #1339

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .jscsrc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"examples/duel/www/**",
"examples/duel/src/main/webapp/js/lib/**",
"examples/polymer/elements/elements.build.js",
"examples/js_of_ocaml/js/*.js",
"**/generated/**"
],
"requireSpaceBeforeBlockStatements": true,
Expand Down
3 changes: 3 additions & 0 deletions examples/js_of_ocaml/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
_build

*.byte
1 change: 1 addition & 0 deletions examples/js_of_ocaml/.jshintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
js/todomvc.js
11 changes: 11 additions & 0 deletions examples/js_of_ocaml/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/sh

# Compile OCaml source file to OCaml bytecode
ocamlbuild -use-ocamlfind \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could use a comment to explain what it does.

-tags "warn(-40)" \
-pkgs lwt.syntax,js_of_ocaml,js_of_ocaml.syntax,js_of_ocaml.tyxml,tyxml,js_of_ocaml.deriving,js_of_ocaml.deriving.syntax,deriving \
-syntax camlp4o \
todomvc.byte ;

# Build JS code from the OCaml bytecode
js_of_ocaml +weak.js --opt 3 -o js/todomvc.js todomvc.byte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a description to the readme about how to bulid the app? That makes it clear for people and helps us to maintain the app further on.

14 changes: 14 additions & 0 deletions examples/js_of_ocaml/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Js_of_ocaml • TodoMVC</title>
<link rel="stylesheet" href="node_modules/todomvc-common/base.css">
<link rel="stylesheet" href="node_modules/todomvc-app-css/index.css">
</head>
<body>
<div id="todomvc"></div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like you are using the 2.0 spec however this root element is an ID, can you switch it to a class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an absolute requirement? The standard way with js_of_ocaml is to select first an element by its id and then use it to plug the application. I could select the <div> element with the getElementsByClassName method but unfortunately this method is for now only available in the development version of js_of_ocaml.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an absolute requirement? I do not think so, however our test suite does a lookup on this element to determine what version of the spec to use when testing, so clearly this does pose some issues.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have an id attribute and a class attribute? ie something like:

<div id="todomvc" class="todoapp"></div>

Otherwise, if the idea is to ban any id attribute, i have no solution for now until the next release of js_of_ocaml (which will include the getElementsByClassName method).

By the way, can you confirm that the needed class name is todoapp? Or is it todomvc? I looked at the file https://github.com/tastejs/todomvc-app-template/blob/master/index.html and i only saw a todoapp class name. Thanks.

<script src="js/todomvc.js"></script>
</body>
</html>
3,289 changes: 3,289 additions & 0 deletions examples/js_of_ocaml/js/todomvc.js

Large diffs are not rendered by default.

Loading