{"action":"created","issue":{"url":"https://api.github.com/repos/typical000/jss-expand/issues/3","repository_url":"https://api.github.com/repos/typical000/jss-expand","labels_url":"https://api.github.com/repos/typical000/jss-expand/issues/3/labels{/name}","comments_url":"https://api.github.com/repos/typical000/jss-expand/issues/3/comments","events_url":"https://api.github.com/repos/typical000/jss-expand/issues/3/events","html_url":"https://github.com/typical000/jss-expand/issues/3","id":165425682,"number":3,"title":"Compatibility with jss 5.1.0","user":{"login":"typical000","id":2428765,"avatar_url":"https://avatars.githubusercontent.com/u/2428765?v=3","gravatar_id":"","url":"https://api.github.com/users/typical000","html_url":"https://github.com/typical000","followers_url":"https://api.github.com/users/typical000/followers","following_url":"https://api.github.com/users/typical000/following{/other_user}","gists_url":"https://api.github.com/users/typical000/gists{/gist_id}","starred_url":"https://api.github.com/users/typical000/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/typical000/subscriptions","organizations_url":"https://api.github.com/users/typical000/orgs","repos_url":"https://api.github.com/users/typical000/repos","events_url":"https://api.github.com/users/typical000/events{/privacy}","received_events_url":"https://api.github.com/users/typical000/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":13,"created_at":"2016-07-13T21:27:42Z","updated_at":"2016-07-31T18:02:53Z","closed_at":null,"body":"Update code to fit jss core **v5.1.0**:\r\n1. Remove arrays concatenation config and functionality. Now it's availablable from jss-core by comma-separated and space-separated syntax (`[5, 10]` and `[[5, 10]]` respectively)\r\n2. As output of plugin must be an array like [['value1', 'value2', 'value3']] that will be transformed to simple string by jss-core. Because we need to handle array inside expanded objects. Now all code is transformed to string directly, and it's a bad idea.\r\n3. Add specific prefix\\syffix\\wrapper for `templates` because there are name collisions with real values and 'placeholders'. E.g. this code will make wrong output:\r\n````````````````js\r\nbackground: {\r\n  image: 'url(image.png)'\r\n}\r\n````````````````\r\nand output will be:\r\n````````````````css\r\nbackground: url(.png);\r\n````````````````\r\nbut must be:\r\n````````````````css\r\nbackground: url(image.png);\r\n````````````````\r\nIt's happening because there is a RegExp that replaces placeholder name with value.\r\nAs a solution we can wrap template parts with **{value}** braces"},"comment":{"url":"https://api.github.com/repos/typical000/jss-expand/issues/comments/236446273","html_url":"https://github.com/typical000/jss-expand/issues/3#issuecomment-236446273","issue_url":"https://api.github.com/repos/typical000/jss-expand/issues/3","id":236446273,"user":{"login":"typical000","id":2428765,"avatar_url":"https://avatars.githubusercontent.com/u/2428765?v=3","gravatar_id":"","url":"https://api.github.com/users/typical000","html_url":"https://github.com/typical000","followers_url":"https://api.github.com/users/typical000/followers","following_url":"https://api.github.com/users/typical000/following{/other_user}","gists_url":"https://api.github.com/users/typical000/gists{/gist_id}","starred_url":"https://api.github.com/users/typical000/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/typical000/subscriptions","organizations_url":"https://api.github.com/users/typical000/orgs","repos_url":"https://api.github.com/users/typical000/repos","events_url":"https://api.github.com/users/typical000/events{/privacy}","received_events_url":"https://api.github.com/users/typical000/received_events","type":"User","site_admin":false},"created_at":"2016-07-31T18:02:53Z","updated_at":"2016-07-31T18:02:53Z","body":"So, now **jss-expand v1.3.0** is realised. What we have:\r\n\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  Array with DIFFERENT sheets: Pure JSS at 20442 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  Array with DIFFERENT sheets: JSS + Plugin at 1194 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  Array with SAME sheets: Pure JSS at 743 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  Array with SAME sheets: JSS + Plugin at 594ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  Bootstrap unnamed JSS test: Pure JSS at 63 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  Bootstrap unnamed JSS test: JSS + Plugin at 41 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  General test with DIFFERENT sheets: Pure JSS at 8929 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  General test with DIFFERENT sheets: JSS + Plugin at 296 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  General test with SAME sheet): Pure JSS at 173 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)  General test with SAME sheet): JSS + Pluginat 140 ops/sec\r\n> Chrome 51.0.2704 (Windows 7 0.0.0)\r\n>\r\n>   Array with DIFFERENT sheets: Pure JSS at 20442 ops/sec (17.11x faster than JSS + Plugin)\r\n>   Array with SAME sheets: Pure JSS at 743 ops/sec (1.25x faster than JSS + Plugin)\r\n>   Bootstrap unnamed JSS test: Pure JSS at 63 ops/sec (1.53x faster than JSS + Plugin)\r\n>   General test with DIFFERENT sheets: Pure JSS at 8929 ops/sec (30.11x faster than JSS + Plugin)\r\n>   General test with SAME sheet): Pure JSS at 173 ops/sec (1.24x faster than JSS + Plugin)\r\n\r\nI just create 5 benchmark tests (4 of this are the the same, but with little difference):\r\n\r\n(\"Pure JSS\" - is test runned with only jss-core inclueded, \"JSS + Plugin\" - is jss with jss-expand)\r\n\r\n* **Array with DIFFERENT sheets** it was simple test where I have tested difference between latest supporting of *comma-separated* and *space-separated* feature from jss core\r\nIn first case (https://github.com/typical000/jss-expand/blob/master/benchmark/fixtures/arrays-without-plugin.json) I have written *comma-separated* and *space-separated* values, supported by jss-core from a box.\r\nIn second case (https://github.com/typical000/jss-expand/blob/master/benchmark/fixtures/arrays-with-plugin.json) I have replaced that 'double brackets' naming with jss-expand feature. Result was:\r\n\r\n> Array with DIFFERENT sheets: Pure JSS at 20442 ops/sec (17.11x faster than JSS + Plugin)\r\n\r\nSo, from this point i conclude that we must rewrite jss-expand algorythm for replacing *double-brakets*. Now it looks like this:\r\n\r\n\r\n```````````````````````js\r\nif (sheme[neededProp]) {\r\n  // If there are simple array like 'margin' or 'padding' - use whitespace\r\n  convertedString = value.join(' ')\r\n}\r\nelse {\r\n  // Otherwise make them comma-separated\r\n  convertedString = value.join(', ')\r\n}\r\n\r\n```````````````````````\r\nThis code was written before jss core v5.0.0 was realised. So this code looks like 'deprecated' feature. But, it's good if we can retain **one syntax** for comma- and space-separated properties.\r\nMaybe I'll try to replace this 'joins' with simple config where are listed properties that must be written in space-separated form, and, if we find this propery just replace 'one-bracket' syntax with 'two-bracket' (just [] => [[]]), and pass this modified code to jss-core after.\r\nI think this solution will be a little faster than previous 'hard-coded' join.\r\n\r\n* **Array with SAME sheets** This test was written to see what is the difference between passing sheet with just jss-core and jss-core + plugin. The difference was:\r\n\r\n>   Array with SAME sheets: Pure JSS at 743 ops/sec (1.25x faster than JSS + Plugin)\r\n\r\nI think this value can be decreased if we implement previouse 'array' optimization, and others.\r\n\r\n* **Bootstrap unnamed JSS test** This test was copy-pasted from jss-core repo and it was dedicated to see difference between pure jss and jss with plugins (It's interesting to see benchmarks like this with other plugins and, maybe with all plugins already created. Maybe we have big performance problems if all plugins are used)\r\n\r\n* **General test with DIFFERENT sheets** In this case i have created 2 different sheets that will be compiled to the same CSS\r\nFirst - without jss-expand (https://github.com/typical000/jss-expand/blob/master/benchmark/fixtures/sheet-without-plugin.json). Here I used just comma- and space-separated features. Other styles was just strings.\r\nSecond - with jss-expand (https://github.com/typical000/jss-expand/blob/master/benchmark/fixtures/sheet-with-plugin.json). In this case\r\n\r\n - Transformed 'double-bracket' syntax to 'single-bracket' ([[]] => [])\r\n - Writtend in 'expanded' way all that I can (backgrounds, box-shadows, transitions, etc.)\r\n\r\nThe result was:\r\n\r\n> General test with DIFFERENT sheets: Pure JSS at 8929 ops/sec (30.11x faster than JSS + Plugin)\r\n\r\nThis is a TERRIFYING number, as for me. Here is the result of using slow 'string replacements'. In this case, I think, we can replace string templates with replacements with creating just an 'double-bracket' array, and passing it to core jss. For example:\r\n\r\n```````````````````````js\r\n// Input:\r\n'box-shadow': {\r\n  x: 0,\r\n  y: 5,\r\n  blur: 15,\r\n  spread: 0,\r\n  color: '#fff'\r\n}\r\n\r\n// Will be converted by jss-expand in this:\r\n'box-shadow' : [[ 0, 5, 15, 0, '#fff' ]]\r\n```````````````````````\r\nThis thins will be more performant. But we must handle in some way **order of properties**. I think we can use as reference, order, in wich properties are written in config https://github.com/typical000/jss-expand/blob/master/src/props.js and sort result before we convert it to double array (need performance test too)\r\n\r\n* **General test with SAME sheet** This test was like **Array with SAME sheets** but a little more expanded. For now nothing interesting. Just a comparsion of jss and jss + plugins.\r\n\r\n\r\n\r\nSo, finalizing all my words:\r\n1. Replace **arrays join** with **array to double array**\r\n2. Replace **string templates and RegExp transforms** with **double array** with some performant sorting algorythm\r\n3. Resolve in some way `font` property exeption, where font-size and line-height are written with 'slash' symbol `size/line-height`. Maybe we'll drop supporting this propery in **jss-expand**. Or, test performance impact, if everytime we will check with `if` - it's a `font` property or not.\r\n\r\n@kof Here all preformance issues described as promised :)"}}
{"actor":{"display_login":"typical000"}}