[DX-399] ES6 template by nickbalestra · Pull Request #252 · opencomponents/base-templates

nickbalestra · 2018-01-28T11:47:57Z

No description provided.

codecov · 2018-02-08T15:39:32Z

Codecov Report

Merging #252 into master will decrease coverage by 0.17%.
The diff coverage is 97.89%.

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
- Coverage   98.68%   98.51%   -0.18%     
==========================================
  Files          31       38       +7     
  Lines         381      472      +91     
  Branches       58       70      +12     
==========================================
+ Hits          376      465      +89     
- Misses          5        7       +2

Impacted Files	Coverage Δ
packages/oc-template-es6-compiler/index.js	`100% <100%> (ø)`
...ckages/oc-generic-template-renderer/lib/getInfo.js	`100% <100%> (ø)`	⬆️
...kages/oc-template-es6-compiler/lib/viewTemplate.js	`100% <100%> (ø)`
...bstract-base-template-utils/webpackConfigurator.js	`100% <100%> (ø)`
packages/oc-template-es6/index.js	`100% <100%> (ø)`
packages/oc-template-es6-compiler/lib/compile.js	`100% <100%> (ø)`
...-base-template-utils/font-family-unicode-parser.js	`100% <100%> (ø)`
...ckages/oc-template-es6-compiler/lib/compileView.js	`95.74% <95.74%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 122b200...229df9d. Read the comment docs.

matteofigus · 2018-02-08T17:51:28Z

@nickbalestra can you check what is making Circle fail?

nickbalestra · 2018-02-08T18:05:33Z

There is no setup for circle on this branch, he tries to enfer some stuff but I think it make sense that it won't be able to run properly

matteofigus

Let's rebase to re-test circle? 👍

nickbalestra · 2018-02-09T12:46:17Z

Rebased! - Cool, its working pretty nice as actually is correct that the node6 build fails but the node8 pass (still waiting for travis.... )

nickbalestra · 2018-02-09T12:51:46Z

All good now :), Out of curiosity I want to see how thing change by checking in the lock file for the cache checksum strategy

nickbalestra · 2018-02-09T13:04:42Z

packages/oc-template-es6/lib/render.js

@@ -0,0 +1,14 @@
+'use strict';
+const es6Renderer = require('express-es6-template-engine');


Note: For getting started I went with this , rationale:

getting up and running quicker,

nice api: (template can simply use ${prop}, without having to do ${model.prop}, good for extendibility

very small code, well tested (the express specific code is very small, so we could think of a fork, in case we want to clean that down in a second step).

allows for some nice thing like precompiling templates (as we do for handlebars), not adding that in in this first iteration but i think is something we might want to explore.

matteofigus · 2018-02-09T16:15:48Z

@nickbalestra not a big fan of the templating choice.
I think the goal here is to have es6. This is not es6, it is a templating language that looks like es6 - which for me is kind of against the whole vision.

To clarify, I think the view should be something like

module.exports = ({ name}) => `<div>${name}</div>`;

A couple of pros:

It is actually javascript. It is simpler and easier to understand.
Dependency free so it's smaller
Allows easy composition (partials can just be required and potentially exposed as npm libraries)
The props.prop vs prop can be elegantly achieved with destructuring
Being javascript, it is cleaner to handle presentation related logic. Example:

module.exports = vm => {
  //code
  return `...`;
}

Views can easily unit tested without being in need of extra tooling (test the compiled version)
We could attach VS debugger - with the template engine, we would need the sourcemaps - not sure that is provided

nickbalestra · 2018-02-09T16:39:20Z

Just fiddling with some ideas :) but I agree with your points.

nickbalestra · 2018-02-11T17:34:05Z

Couple of thing I also want achieve with this (still in a wip at the moment).

Opinionated on css-in-js exactly in the same way we doing with the react-template
allow webpack to bundle/compile the view in case you adding more stuff to it

nickbalestra · 2018-02-22T14:18:36Z

@matteofigus This should be now ready to review

nickbalestra · 2018-02-22T14:23:04Z

PS: Once this get merged, I'll rebase all the other outstanding PRs

… yarn workspace hoisting changes

nickbalestra · 2018-02-23T11:38:43Z

@matteofigus is now rebased to master

matteofigus

can we also remove the yarn.lock?

matteofigus · 2018-03-01T12:55:14Z

packages/oc-template-es6-compiler/lib/viewTemplate.js

+      'window.oc = window.oc || {};' +
+      'oc.cmd = oc.cmd || [];' +
+      'oc.cmd.push(function(oc){' +
+        'oc.addStylesToHead(\\'${css}\\');' +


should we move to <style> + event to keep this consistent with the React one?

matteofigus · 2018-03-01T12:56:01Z

packages/oc-template-es6-compiler/test/compileView.test.js

@@ -0,0 +1,51 @@
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 50000;
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 50000;


why is this timeout here twice?

matteofigus

Just a question

matteofigus · 2018-03-02T00:04:42Z

packages/oc-template-es6-compiler/test/compile.test.js

@@ -0,0 +1,3 @@
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 100000;
+test('', () => {});
+// WIP


This still needs to be completed?

matteofigus

As discussed, this is looking good but we want to handle the empty CSS scenario (similar to React) for when the CSS is not used.

nickbalestra · 2018-03-05T14:08:53Z

@matteofigus give it a look, In this case was possible to reduce the template even further, let me know what ya think

matteofigus · 2018-03-05T16:35:52Z

packages/oc-template-es6-compiler/lib/viewTemplate.js

+          'window.oc = window.oc || {};' +
+          'oc.cmd = oc.cmd || [];' +
+          'oc.cmd.push(function(oc){' +
+          "oc.events.fire(\\'oc:cssDidMount\\', \\'${css}\\');" +


${css} this should be outside of that

nickbalestra added the wip label Jan 28, 2018

nickbalestra force-pushed the es6-template branch from 126f229 to 5d1f8c5 Compare February 8, 2018 15:39

matteofigus requested changes Feb 8, 2018

View reviewed changes

nickbalestra force-pushed the es6-template branch from 5d1f8c5 to a848ad3 Compare February 9, 2018 12:43

nickbalestra changed the title ~~ES6 template~~ [WIP] ES6 template Feb 9, 2018

nickbalestra changed the title ~~[WIP] ES6 template~~ [WIP][DX-399] ES6 template Feb 9, 2018

nickbalestra removed the wip label Feb 9, 2018

nickbalestra commented Feb 9, 2018

View reviewed changes

nickbalestra changed the title ~~[WIP][DX-399] ES6 template~~ [DX-399] ES6 template Feb 9, 2018

nickbalestra changed the title ~~[DX-399] ES6 template~~ [WIP][DX-399] ES6 template Feb 9, 2018

nickbalestra changed the title ~~[WIP][DX-399] ES6 template~~ [DX-399] ES6 template Feb 22, 2018

nickbalestra mentioned this pull request Feb 22, 2018

V1 release prep work opencomponents/oc#833

Open

4 tasks

Nick Balestra added 8 commits February 23, 2018 11:26

init wip base for es6 template

1cbc1e3

dependencies.io setup

5956969

readme setup

45fc9b6

template index

bdffda7

wip

119e9da

oc-template-es6 wip

bbcdada

oc-template-es6 compiler wip

2c5ad9d

node6 fix

4c36889

Nick Balestra and others added 13 commits February 23, 2018 11:26

wip to support css modules as we do within the react template

927229f

updated oc

4400b4b

updated compileView & viewTemplate to support csshandling

2b7f25b

acceptance component

91e3aa5

updated other acceptance components to point to local compiler due to…

323a799

… yarn workspace hoisting changes

japanese character used in font-family for test

6f36db1

acceptance setup to test es6 template as well

1fc4a1a

node6 safe

95168f6

updated latest logo on templates

1eea843

added scaffold component to be used by the cli

ad7ef2b

small updates

9ea7422

updated snaps

03b4ac1

yarn.lock

423ae27

nickbalestra force-pushed the es6-template branch from 022ca31 to 423ae27 Compare February 23, 2018 11:32

matteofigus requested changes Mar 1, 2018

View reviewed changes

nickbalestra added 2 commits March 1, 2018 14:05

fixes

45d1dd6

snaps++

244b158

matteofigus approved these changes Mar 2, 2018

View reviewed changes

tests and fixes

b6c885e

matteofigus requested changes Mar 5, 2018

View reviewed changes

css-less template

f7cb50a

snaps

13f29f6

matteofigus reviewed Mar 5, 2018

View reviewed changes

nickbalestra added 2 commits March 5, 2018 16:38

template

6555b4d

template

229df9d

matteofigus approved these changes Mar 5, 2018

View reviewed changes

matteofigus merged commit b01c11e into master Mar 5, 2018

matteofigus deleted the es6-template branch March 5, 2018 16:51

		@@ -0,0 +1,14 @@
		'use strict';
		const es6Renderer = require('express-es6-template-engine');

		@@ -0,0 +1,51 @@
		jasmine.DEFAULT_TIMEOUT_INTERVAL = 50000;

		jasmine.DEFAULT_TIMEOUT_INTERVAL = 50000;

Conversation

nickbalestra commented Jan 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

matteofigus commented Feb 8, 2018

Uh oh!

nickbalestra commented Feb 8, 2018

Uh oh!

matteofigus left a comment

Choose a reason for hiding this comment

Uh oh!

nickbalestra commented Feb 9, 2018

Uh oh!

nickbalestra commented Feb 9, 2018

Uh oh!

nickbalestra Feb 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matteofigus commented Feb 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickbalestra commented Feb 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickbalestra commented Feb 11, 2018

Uh oh!

nickbalestra commented Feb 22, 2018

Uh oh!

nickbalestra commented Feb 22, 2018

Uh oh!

nickbalestra commented Feb 23, 2018

Uh oh!

matteofigus left a comment

Choose a reason for hiding this comment

Uh oh!

matteofigus Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

nickbalestra Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

matteofigus Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

matteofigus left a comment

Choose a reason for hiding this comment

Uh oh!

matteofigus Mar 2, 2018

Choose a reason for hiding this comment

Uh oh!

nickbalestra Mar 2, 2018

Choose a reason for hiding this comment

Uh oh!

matteofigus left a comment

Choose a reason for hiding this comment

Uh oh!

nickbalestra commented Mar 5, 2018

Uh oh!

matteofigus Mar 5, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nickbalestra commented Jan 28, 2018 •

edited

Loading

codecov bot commented Feb 8, 2018 •

edited

Loading

nickbalestra Feb 9, 2018 •

edited

Loading

matteofigus commented Feb 9, 2018 •

edited

Loading

nickbalestra commented Feb 9, 2018 •

edited

Loading