-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
Conversation
Personally I think should totally go discuss this at TasteLang but that it isn't really a good fit for TodoMVC. Mainly because this is more about the language than about an MV* framework. What does the rest think? |
OCaml has a really nice JS story these days and I hear it more and more, especially among people looking for a type-safe way to write web apps, where there aren't very many options out there. The implementation does a pretty nice job of separating the concerns and looks pretty similar to the Elm way, so I'm 👍. |
|
||
js_of_ocaml +weak.js --opt 3 -o js/todomvc.js todomvc.byte | ||
|
||
# Avoid JS linters (hard to apply for auto generated JS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this magic just ignore specific files in the config files.
Any active community member who's able to review the code? I don't think any of us is (or am I wrong?) |
I'm at a read-only level of OCaml understanding. I'll give it a more On Fri, Jun 26, 2015, 22:49 Arthur Verschaeve [email protected]
|
The code has been reviewed by @Drup who is a member of the jsoo project. The code style is now much better. |
Oh yes, I prefer the new style a lot over the old |
@@ -0,0 +1,2 @@ | |||
_build | |||
*.byte |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing a newline
Neat work. Thanks. |
|
||
Various code improvements from [Gabriel Radanne](https://github.com/Drup). | ||
|
||
Based on [Elm implementation](https://github.com/evancz) by Evan Czaplicki. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this a link on the name itself
Thanks for your nice work! |
<link rel="stylesheet" href="node_modules/todomvc-app-css/index.css"> | ||
</head> | ||
<body> | ||
<div id="todomvc"></div> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
we look for For now doing both is an OK solution, thanks for chatting with me, and thanks for the awesome work. ps I tried compiling this app locally by following your instructions and ran into all kinds of errors when installing dependencies, Do you think you might be able to expand the setup instructions for how to compile this app, thanks! |
I added the "todoapp" class. Thank you for your feedback about your attempt to compile the application. I made a test by starting from a clean environment. And indeed it's not as simple as it seems... So i updated the building instructions. I think it's a lot better now. |
looks like we are in need of a rebase 🌐 |
After some fixes, there are still 3 failed checks. They are all from the routing part and they all fail because of "stale element reference: element is not attached to the page document". Not sure if i can apply some workarounds? |
@slegrand45 how recently have you rebased off of master? We Very recently merged a fix #1371 |
Seems like some unrelated commits got into this PR. Can you rebase and get them out? |
0b90397
to
df745b3
Compare
It's strange because i already made a rebase... Anyway, i used the following commands: git checkout js_of_ocaml
git fetch upstream
git rebase upstream/master
git push -f Strangely enough, i had several conflicts about my own commits during the rebase (?). I used --skip to resolve them. I had also to use "push -f" because git said that my local branch and "origin/js_of_ocaml" have diverged (?). It seems that the PR is now clean again. But i hope i didn't introduce some mess 😟 Besides, the CI log still shows some "stale element reference" (?). |
yep @slegrand45 looks like you uncovered some more flake 👏 |
hey @slegrand45 would you mind rebasing this off of master and squashing your commits down? |
Js_of_ocaml is a compiler of OCaml bytecode to Javascript. OCaml is a general purpose programming language with an emphasis on expressiveness and safety. Js_of_ocaml brings nearly all the OCaml features to Javascript. And especially its powerful type system.
df745b3
to
b6d468f
Compare
Seems ok! |
@passy I will let you have final 👍 on this |
@slegrand45 I'm seeing some odd rendering errors among other things. Is the latest version on your branch up to date? |
@passy Yes it's the latest version. What are the errors you are seeing? |
Rendering is broken like on the screenshot and I've only tried a few things that all weren't quite right, focus lost after creating an item, no selection when entering edit mode etc. Do you see those things on your end too? |
Ok, i see what you mean. Indeed, i have the same problems. I'm working on it right now to fix it. |
@passy I think it should be better now. |
@slegrand45 Oh yes, way better! 🌵 I'm checking it now. :) |
I took me way longer than it should have to find this GIF, but it was worth it and I'm super excited about landing this. Thanks so much @slegrand45! Also thanks @arthurvr and @samccone for the review! 😻 |
Thank you very much to all the team! 😃 |
Hello everyone, note sure if it's the right place to ask this :/ Do you have an example with version of dependencies ? When I'm trying with latest opam,eliom config I have problem with js_of_ocaml.deriving.syntax not found. I've tried many different ways and I'm still stucked.. Thx for yout help. 😄 |
Hi, Indeed, some packages have been renamed since 2015. @LouisAyroles I have sent a PR to fix that: #2155. You can also take a look at the examples at https://github.com/slegrand45/examples_ocsigen. These examples should compile and run without any problem. If not, please send an issue report. |
Js_of_ocaml is a compiler of OCaml bytecode to Javascript.
OCaml is a general purpose programming language with an emphasis on expressiveness and safety. Js_of_ocaml brings nearly all the OCaml features to Javascript. And especially its powerful type system.