{"action":"created","issue":{"url":"https://api.github.com/repos/posthtml/posthtml/issues/159","repository_url":"https://api.github.com/repos/posthtml/posthtml","labels_url":"https://api.github.com/repos/posthtml/posthtml/issues/159/labels{/name}","comments_url":"https://api.github.com/repos/posthtml/posthtml/issues/159/comments","events_url":"https://api.github.com/repos/posthtml/posthtml/issues/159/events","html_url":"https://github.com/posthtml/posthtml/pull/159","id":169005709,"number":159,"title":"v1.0 Proposed Changes","user":{"login":"jescalan","id":556932,"avatar_url":"https://avatars.githubusercontent.com/u/556932?v=3","gravatar_id":"","url":"https://api.github.com/users/jescalan","html_url":"https://github.com/jescalan","followers_url":"https://api.github.com/users/jescalan/followers","following_url":"https://api.github.com/users/jescalan/following{/other_user}","gists_url":"https://api.github.com/users/jescalan/gists{/gist_id}","starred_url":"https://api.github.com/users/jescalan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jescalan/subscriptions","organizations_url":"https://api.github.com/users/jescalan/orgs","repos_url":"https://api.github.com/users/jescalan/repos","events_url":"https://api.github.com/users/jescalan/events{/privacy}","received_events_url":"https://api.github.com/users/jescalan/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":29,"created_at":"2016-08-02T22:38:54Z","updated_at":"2016-08-09T17:25:59Z","closed_at":"2016-08-09T17:24:54Z","pull_request":{"url":"https://api.github.com/repos/posthtml/posthtml/pulls/159","html_url":"https://github.com/posthtml/posthtml/pull/159","diff_url":"https://github.com/posthtml/posthtml/pull/159.diff","patch_url":"https://github.com/posthtml/posthtml/pull/159.patch"},"body":"Ok, this is a really large pull request, so be warned that this description is going to be long and very thorough so I have the opportunity to clearly explain everything.\r\n\r\nThe essential purpose of this PR is to complete the objectives proposed in #149. As it stands, all of them have been accomplished, and tested across a decent number of plugins. So let's go over what has changed here.\r\n\r\n## New AST Format\r\n\r\nFirst and most important change is that [the posthtml AST is different now](https://github.com/static-dev/posthtml#posthtml-ast). This is a **breaking change** for every single plugin, as well as the posthtml core, parser, and renderer (and all of these have been rewritten to adapt to the new format). Now, this is a large change, so let me justify why it was made.\r\n\r\n### Location Information\r\n\r\nIn order to correctly report errors and produce source maps, each AST node needs *location* information -- that is, we need to map back to a line and column in the original source. For example, if you were working with the `posthtml-include` plugin, and the user had some code like this:\r\n\r\n```html\r\n<p>before include</p>\r\n<include source='foo/bar.html'>\r\n<p>after include</p>\r\n```\r\n\r\nThe plugin actually uses `src` instead of `source`, so this code would be incorrect. As such, it's the plugin's responsibility to throw an error, maybe something like `include tag missing \"src\" attribute`. However, on a site with many includes, this could be very difficult to track down. A much better error would be something like this:\r\n\r\n```\r\nError: \"include\" tag missing \"src\" attribute\r\nLocation: /Users/someone/project/index.html:2:9\r\nPlugin: posthtml-include\r\n\r\n  1 | <p>before include</p>\r\n> 2 | <include source='foo/bar.html'>\r\n    |          ^\r\n  3 | <p>after include</p>\r\n```\r\n\r\nThis is the type of error that can be produced with accurate location information, and this is awesome. It helps users to quickly understand the error, and exactly where it is in their code. This is incredibly important to have in order to make posthtml a popular and production-ready library. This pull request makes it such that [plugins can produce this type of error](https://github.com/static-dev/posthtml/blob/master/lib/error.js).\r\n\r\nAdditionally, this information enables developers to use sourcemaps. Sourcemaps could be produced by a plugin made by a third party -- I know that none of the core developers have much of an interest in sourcemaps, but as mentioned earlier, there's already [an open issue requesting them](https://github.com/posthtml/posthtml/issues/138), and more and more requests will start coming in as soon as posthtml becomes more popular, so it's important to at least give users the information needed to produce them.\r\n\r\nNow, why couldn't the old AST be used, but just add location information? In the old AST, text nodes were represented by pure strings, which cannot have properties. Therefore, adding location info to nodes was impossible with the old AST, meaning a breaking change was necessary completely.\r\n\r\n## New Parser\r\n\r\nIn order to generate this new AST format, the parser needed to be rewritten. However, htmlparser2 does not provide location information, which means that it could not be used. Building a new parser has been discussed at length in the posthtml channel, however, building a full-featured html parser is quite a difficult task. As such, I rebuilt the parser based on [parse5](https://github.com/inikulin/parse5), a robust and mature html parser than has proven itself in production. This parser works very well, and the new parser implementation is extremely short, under 200 lines, and includes full location info.\r\n\r\nThere is [only one issue](https://github.com/inikulin/parse5/issues/144) we are running up against at the moment, although the maintainers do have plans of resolving it, and I have an idea for a patch in the meantime.\r\n\r\n## Template Generation\r\n\r\nI am aware that you guys at theprotein.io like to use a BEM template engine for your front-end templates, however, not everyone uses BEM, and many people would like to have a more flexible front-end templating system available. It's no surprise that using html templates on the front-end is extremely common (backbone, vue, react, angular, pretty much any front end framework uses them) and often necessary when building single page apps, so adding this capability to posthtml is something that would benefit many.\r\n\r\nIn order for this to happen, a new code generator was necessary, since the current code generator produces a string instead of a function. So to try to clarify the difference, given the following template, which uses `posthtml-exp`:\r\n\r\n```html\r\n<p>Hello {{ planet }}!</p>\r\n```\r\n\r\nWith the current version of posthtml, we would get the following output, assuming that we run it with `{ locals: { planet: 'world' } }`:\r\n\r\n```js\r\n\"<p>Hello world!</p>\"\r\n```\r\n\r\nHowever, with the new code generator, we're looking at something like the following output:\r\n\r\n```js\r\nfunction (locals) {\r\n  return \"<p>Hello\" + locals.planet + \"!</p>\"\r\n}\r\n```\r\n\r\nThis way, you can load this function up into a javascript app and produce html on demand. For example, if you had a to-do list, you could repeatedly render the same template with different items. And this is just a simple example, just posthtml-exp also contains loops and conditionals, and other plugins have limitless capacity for adding interactivity to templates like this.\r\n\r\nIn order to accommodate this change, a new type of node was added to the AST, `code`. This type of node represents code that will be evaluated at runtime rather than compile-time. Posthtml expressions has been tested with fairly complex `code` nodes, loops, and if/else statements, and is producing accurate html.\r\n\r\nI also have added the concept of a `runtime object`, which is essentially a set of utility functions to be made available at runtime. Read more about the runtime object [here](https://github.com/static-dev/posthtml#the-runtime)\r\n\r\n## Code Quality\r\n\r\nDefinitely more of a side benefit than a primary goal, but all of the projects that were touched (core, parser, generator, sugarml, plugin-util, six plugins) all use the same very consistent tools and code style, are fully tested and documented, the readme style is consistent, and all are at 100% code coverage.\r\n\r\n# The Changes\r\n\r\nThe entirety of posthtml core, the default parser, the default generator, and the sugarml parser were totally rewritten to accommodate this change. You can see these changes here:\r\n\r\n- [core](https://github.com/static-dev/posthtml)\r\n- [parser](https://github.com/static-dev/posthtml-parser2)\r\n- [code generator](https://github.com/static-dev/posthtml-code-gen)\r\n- [sugarml](https://github.com/posthtml/sugarml/tree/refactor)\r\n\r\nIn addition, I have created a [plugin utility](https://github.com/static-dev/posthtml-plugin-util) that includes a couple tools that make it easier to work with the AST inside a plugin, and updated a large portion of the essential core plugins to accommodate the new API:\r\n\r\n- [posthtml expressions](https://github.com/posthtml/posthtml-expressions/tree/gen-function)\r\n- [posthtml content](https://github.com/posthtml/posthtml-content/tree/refactor)\r\n- [posthtml extend](https://github.com/static-dev/posthtml-extend)\r\n- [posthtml include](https://github.com/posthtml/posthtml-include/tree/refactor)\r\n- [posthtml retext](https://github.com/jescalan/posthtml-retext)\r\n- [posthtml custom elements](https://github.com/posthtml/posthtml-custom-elements/tree/refactor)\r\n\r\nOf course, more plugins could be updated, however, I wanted to just get a decent number of them done in order to test the new API and to serve as a proof of concept that this thing works. And it does. The tests and coverage are there, all that remains is to test this out in some production projects!\r\n\r\nI know this is a big change, but I think it will be a big step forward for posthtml. It will still easily be able to generate static html as before, but it will also now allow for a large variety of new use cases and a new class of plugins that will fill in a lot of peoples' use cases. Happy to field any questions about this!"},"comment":{"url":"https://api.github.com/repos/posthtml/posthtml/issues/comments/238626545","html_url":"https://github.com/posthtml/posthtml/pull/159#issuecomment-238626545","issue_url":"https://api.github.com/repos/posthtml/posthtml/issues/159","id":238626545,"user":{"login":"michael-ciniawsky","id":5419992,"avatar_url":"https://avatars.githubusercontent.com/u/5419992?v=3","gravatar_id":"","url":"https://api.github.com/users/michael-ciniawsky","html_url":"https://github.com/michael-ciniawsky","followers_url":"https://api.github.com/users/michael-ciniawsky/followers","following_url":"https://api.github.com/users/michael-ciniawsky/following{/other_user}","gists_url":"https://api.github.com/users/michael-ciniawsky/gists{/gist_id}","starred_url":"https://api.github.com/users/michael-ciniawsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/michael-ciniawsky/subscriptions","organizations_url":"https://api.github.com/users/michael-ciniawsky/orgs","repos_url":"https://api.github.com/users/michael-ciniawsky/repos","events_url":"https://api.github.com/users/michael-ciniawsky/events{/privacy}","received_events_url":"https://api.github.com/users/michael-ciniawsky/received_events","type":"User","site_admin":false},"created_at":"2016-08-09T17:25:59Z","updated_at":"2016-08-09T17:25:59Z","body":"> @jescalan @michael-ciniawsky to many things for discussion. AST format from @jescalan will not be included in the 1.0.0 scope. Can we discuss later in the different PR? So, a few days ago I asked about smaller PR's. I don't see them now. \r\n\r\nYes, i understand that, but in someway we must find way to propose changes to core iself, discuss the eventually integration, when you're fine with them and finally how to do it or how you are willing to accept them. There are few justifiable improvements lurking around #101, #159 in my opinion, but they have their smaller flaws aswell. The tension to write your __own__ personal bias flavoured posthtml core lib currently points in a wrong/bad direction in general, that's what plugins are made for :). We maybe rushed things to fast and get a little bit cocky here, but there needs to be detail feedback from your (core devs) side on what is proposed, why not, basically yes but not in this concrete way, not know but... etc. E.g the hole AST thing could have been a PR to posthtml-parser, there is no need to __rewrite__ the core lib, what i criticize is that there is no 'guidance', thats a clear factor why everyone is cooking their own core implementations, while on the other hand they should develop plugins for there personal preferences instead. If a major version with eventual breaking changes is intended/needed(Everything fine, need is not really right here, but a few smaller things even when breaking could/should be done before a major bump),  thats an important decision affecting everyone, which needs to be well thought out not having this kind of discussion every week and a good standing point to simply say 'Write a plugin ;)'?"}}
{"actor":{"display_login":"michael-ciniawsky"}}