Boilerplate refactor #8820

stevenhao · 2017-06-19T19:50:45Z

Summary of changes

This PR refactors the boilerplate generator to generate the html string using javascript instead of using Blaze with Spacebars templates. Except for some minor differences (whitespace, html-escaping), the generated html is the same as it was before the refactor.

Changes to the boilerplate-generator package:

Bump version to 1.1.2
Rename and modernize boilerplate-generator.js to generator.js
Delete boilerplate_web.browser.html and boilerplate_web.cordova.html
Add template_web.browser.js and template_web.cordova.js

This PR also adds a boilerplate-generator-tests package (previously, all the boilerplate-related tests were tested indirectly in the webapp package).

apollo-cla · 2017-06-19T19:50:46Z

@stevenhao: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

glasser · 2017-06-19T22:47:28Z

packages/boilerplate-generator/boilerplate-generator.js

@@ -11,11 +11,9 @@ Boilerplate = function (arch, manifest, options) {
  options = options || {};


This is really more of a question for @benjamn or @abernix but should new code be written with ES6 these days? import instead of Npm.require (it's ok even in packages to use import for built in packages like this one, right?), no var, class, etc? Using mainModule and imports instead of file lists and package-local variables?

And specifically should this refactored class be rewritten to new style or left alone?

Let's modernize this code! It runs on the server anyway. Specifically, @stevenhao, we should put api.use("ecmascript", "server") in package.js, define an api.mainModule, and use class syntax (etc.) in this file.

glasser · 2017-06-19T22:48:15Z

packages/boilerplate-generator/boilerplate-generator.js

-  return  "<!DOCTYPE html>\n" +
-    Blaze.toHTML(Blaze.With(_.extend(self.baseData, extraData),
-                            self.func));
+  return  "<!DOCTYPE html>\n" + self.template(_.extend(self.baseData, extraData));


_.extend modifies self.baseData in place — is that intentional? I think maybe _.extend({}, self.baseData, extraData) may be better. I see that's in the existing code too but maybe that's a bug.

Here's an idea you can ignore for the time being:

I would love to enable more interesting extensions of self.baseData here, specifically to support server-side rendering. For example, if this code called some custom callback with self.baseData, then the callback could parse the self.baseData.body HTML and attach server-rendered fragments to DOM nodes with particular ids. Then the client could use document.getElementById to find the same nodes and rehydrate the server-generated HTML.

For now, let's focus on feature parity, but I wanted to plant that seed. 🌱

yup, i'll change it to _.extend({}, self.baseData, extraData), or if it's safe to use ES6, Object.assign({}, self.baseData, extraData)

ES6 is definitely fine in server code and I think in client code too.

glasser · 2017-06-19T22:50:37Z

packages/boilerplate-generator/boilerplate_web_template.js

+// XXX Does not necessarily preserve formatting (e.g. additionalStaticJs newlines)
+// Arguments: root : { htmlAttributes, css : [{ url }], bundledJsCssUrlRewriteHook : Function, head, dynamicHead, body, dynamicBody, inlineScriptsAllowed, additionalStaticJs, meteorRuntimeConfig }
+
+Boilerplate_Web_Browser_Template = function(root) {


This isn't our normal variable naming style. I think BoilerplateWebBrowserTemplate, or even just WebBrowserTemplate since we're not exporting it out of the package.

glasser · 2017-06-19T22:51:53Z

packages/boilerplate-generator/boilerplate_web_template.js

+
+    // XXX is htmlAttributes ever anything but {}?
+    // may just be a generic Blaze/Spacebars thing.
+    function props1(htmlAttributes) {


I don't think you need to name these functions that are only called immediately. But I also am unsure why this is necessary — can't you just write root.htmlAttributes directly? Is this based on starting with some generated code? I'd simplify this to look more like functions you'd write yourself.

glasser · 2017-06-19T22:57:24Z

packages/boilerplate-generator/boilerplate_web_template.js

+      return [
+        ['<html'].concat(Object.keys(htmlAttributes).map(function(key) {
+
+          // XXX probably need to wrap strings in "".


Definitely want to have some escaping here! Not just wrapping, also doing proper encoding

Yeah, I've been delaying implementing this because I'm not sure what the best library for this kind of stuff.

From what I can tell after reading https://github.com/meteor/blaze/blob/master/packages/htmljs/visitors.js#L198, https://html.spec.whatwg.org/multipage/syntax.html#attributes-2,

an attribute pair should stringify to _.template(' <%= key %>="<%- value %>"')({key, value}).

Potential issues:

The attribute namespace isn't explicitly handled and must be directly attached to the key of htmlAttributes, e.g. htmlAttributes may need to have entries like {"xmlns:h": "http://www.w3.org/2000/xmlns/"}.

_.template("<%- ... %>") does HTML escaping, does not distinguish between attribute mode. this project has an isAttributeValue option that may be useful. It seems to me that the attribute mode distinction isn't hugely important, and that most browsers handle unnecessarily escaped characters gracefully.

we chatted about this in person and decided that we need to HTML-encode attribute values but not names, and don't need to URL-encode anything.

glasser · 2017-06-19T22:58:30Z

packages/boilerplate-generator/boilerplate_web_template.js

+      ''
+    ],
+
+    function if1(inlineScriptsAllowed) {


this one might be easier just as a ?: thing?

glasser · 2017-06-19T23:00:18Z

packages/boilerplate-generator/boilerplate_web_template.js

+      var bundledJsCssUrlRewriteHook = root.bundledJsCssUrlRewriteHook;
+      return _.map(css, function(obj) {
+        var url = obj.url;
+        return '  <link rel="stylesheet" type="text/css" class="__meteor-css__" href="' + bundledJsCssUrlRewriteHook(url) + '">';


url needs escaping (anything in the original template that's {{}} rather than {{{}}} should be escaped). Note that escaping is either URL-escaping or HTML-escaping depending on where it is, I think?

glasser · 2017-06-19T23:01:30Z

packages/boilerplate-generator/package.js

+
+Package.onUse(function (api) {
+  api.use([
+    'underscore',


Given that we're relying on underscore anyway, maybe just use _.template with a template stored in a file and read by Assets.getText?

I had no idea this existed! I'll try it out.

benjamn · 2017-06-22T22:16:28Z

packages/boilerplate-generator/boilerplate-generator.js

-var fs = Npm.require('fs');
-var path = Npm.require('path');
+import fs from 'fs';
+import path from 'path';


I would be more specific:

import { readFile } from "fs"; import { ??? } from "path";

It looks like you're not using path anywhere in this file, so I think you can remove that line.

benjamn · 2017-06-22T22:17:24Z

packages/boilerplate-generator/boilerplate-generator.js

-  );
-};
+export class Boilerplate {
+  constructor (arch, manifest, options = {}) {


We don't generally put a space after method names (before the ().

benjamn · 2017-06-22T22:17:50Z

packages/boilerplate-generator/boilerplate-generator.js

-    Blaze.toHTML(Blaze.With(_.extend(self.baseData, extraData),
-                            self.func));
-};
+    return  "<!DOCTYPE html>\n" + this.template(Object.assign({}, this.baseData, extraData));


This is pretty long line that might be worth breaking after the +, as before.

benjamn · 2017-06-22T22:19:24Z

packages/boilerplate-generator/boilerplate_web_cordova_template.js

+// Replicates the template defined in boilerplate_web.cordova.html
+// Arguments: root : { htmlAttributes, css : [{ url }], bundledJsCssUrlRewriteHook : Function, head, dynamicHead, body, dynamicBody, inlineScriptsAllowed, additionalStaticJs, meteorRuntimeConfig }
+
+export default function(manifest) {


We do generally put a space after the function keyword (before the () for anonymous functions.

benjamn · 2017-06-22T22:20:31Z

packages/boilerplate-generator/boilerplate-generator.js

-    // XXX the template we are evaluating relies on the fact that UI is globally
-      // available.
-    global.UI = UI;
-    self.func = eval(boilerplateRenderCode);


Glad to see this eval go away!

benjamn · 2017-06-22T22:22:32Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+      '</html>'
+    ],
+
+    ['', '<!-- Generated for cordova by boilerplate-generator -->']


Should this be referring to Cordova, if this is boilerplate_web_browser_template.js?

glasser · 2017-06-24T05:31:23Z

packages/boilerplate-generator/boilerplate-generator.js

-  return Assets.getText(filename);
+// Returns a template function that, when called, produces the boilerplate
+// html as a string.
+const _getTemplate = _.memoize(arch => {


I don't think the memoize is necessary anymore now that the function isn't doing disk IO.

glasser · 2017-06-24T05:31:57Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+// Arguments: root : { htmlAttributes, css : [{ url }], bundledJsCssUrlRewriteHook : Function, head, dynamicHead, body, dynamicBody, inlineScriptsAllowed, additionalStaticJs, meteorRuntimeConfig }
+
+export default function (manifest) {
+  const root = manifest;


Not sure why you're renaming the parameter instead of just giving it whatever name you want it to have.

glasser · 2017-06-24T05:32:33Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+    [
+      '<html' +_.map(root.htmlAttributes, (value, key) =>
+        _.template(' <%= attrName %>="<%- attrValue %>"')({
+          attrName: key,


A little confusing to choose to call it "key" and then a line later call the same thing "name"

I agree that it's a bit confusing, but I was hoping that it would make sense:

(value, key) correspond to {key: value} pairs

attrName and attrValue are are templated as name="value" HTML attributes.

I'm happy to change it to be '<% key %>="<%- value %>"' if you think that's less confusing.

glasser · 2017-06-24T05:35:45Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+        ? '  <script type="text/javascript"><%= contents %></script>'
+        : '  <script type="text/javascript" src="<%- src %>"></script>'
+      )({
+        contents: _.template('__meteor_runtime_config__ = JSON.parse(decodeURIComponent(<%= conf %>))')({


Not sure why you're doing nested calls to _.template instead of just putting this in the first template. Also, it's kinda weird that you're providing both keys (contents/src) to either template... why not do

root.inlineScriptsAllowed ? _.template('template1', args1) : _.template('template2', args2)

?

Also in the old version, meteorRuntimeConfig was {{}} (HTML-escaped) though it probably doesn't matter due to the use of encodeURIComponent.

glasser · 2017-06-24T05:37:14Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+        contents: _.template('__meteor_runtime_config__ = JSON.parse(decodeURIComponent(<%= conf %>))')({
+          conf: root.meteorRuntimeConfig
+        }),
+        src: root.rootUrlPathPrefix + '/meteor_runtime_config.js'


the second half here can just go in the template...

glasser · 2017-06-24T05:40:03Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+    ),
+
+    _.map(root.additionalStaticJs, ({contents, pathname}) => (
+      _.template(root.inlineScriptsAllowed


similarly, this should probably be separate template calls

glasser · 2017-06-24T05:40:26Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+
+    _.map(root.additionalStaticJs, ({contents, pathname}) => (
+      _.template(root.inlineScriptsAllowed
+        ? '  <script><%= contents %></script>'


Looks like contents should actually be escaped.

It seems like we have to be careful with over-escaping contents here, because some browsers don't handle escaping very well when it's within a script tag.

This Gist shows how unexpected behavior can occur, even on master.

Discussed this with @benjamn, we decided that it's probably safest to load all additionalStaticJs from the server. The reason is that embedding arbitrary javascript inline is tricky to do correctly, and can cause strange bugs.

The tradeoff we're making here is that loading from the server may be more costly than inlining the script. The only internal package that is loaded through additionalStaticJs is https://github.com/meteor/meteor/tree/master/packages/reload-safetybelt, and as far as I'm aware, external packages don't really use WebAppInternals.addStaticJs. Correct me if I'm wrong, but it seems like making this change won't be too disruptive?

Hmm, I mean, this was an explicit choice to minimize round trips (ie, startup time) for apps, and I think it's been working for many years? If it's only used by reload-safetybelt, a weird package we don't really even recommend, then I guess it's not a big deal, but in that case maybe it's not a big deal in either direction (if we are confident that reload-safetybelt's code is escaped correctly)?

glasser · 2017-06-24T05:44:41Z

packages/boilerplate-generator/boilerplate_web_cordova_template.js

+    ),
+    _.map(root.additionalStaticJs, ({pathname, contents}) =>
+      _.template(inlineScriptsAllowed
+        ? '  <script type="text/javascript"><%= contents %></script>'


escape, use multiple templates.

Although honestly I'm not sure why we check for inlineScriptsAllowed here when there's already a giant inline script above. Seems like a bug. Not sure who actually knows about this stuff anymore — overlap of the browser content policy package and Cordova. @benjamn ? @martijnwalraven ?

I'm going to err on the side of caution and not change the behavior of this section. If we decide that we'll be inlining all scripts for cordova, or inline no scripts for browser (or both), I think that can be a separate PR.

glasser · 2017-06-24T05:45:02Z

packages/boilerplate-generator/boilerplate_web_cordova_template.js

+      '</html>'
+    ],
+
+    ['', '<!-- Generated for cordova by boilerplate-generator -->']


Are these comments going to stick around forever?

I thought it might be nice to leave a note in generated code about how it was generated, but maybe it doesn't make sense to expose this to the public

@stevenhao

This is a bug that will be fixed by @stevenhao's boilerplate-generator refactoring (#8820), but I need it fixed now :)

@stevenhao

This is a bug that will be fixed by @stevenhao's boilerplate-generator refactoring (#8820), but I need it fixed now :)

glasser · 2017-06-27T23:52:34Z

Looking generally good. I'll leave what to do about additionalStaticJs up to Meteor team. This is the last feedback I can give until back from vacation next Wednesday — I'm sure Meteor team can help you finish this one up :)

@stevenhao

This is a bug that will be fixed by @stevenhao's boilerplate-generator refactoring (#8820), but I need it fixed now :)

This reverts commit 54fe867.

abernix

Awesome! I think functionally this looks great (and works!), but I've added my comments below, some of which are worth addressing. Mostly nitpicks and formatting though!

abernix · 2017-07-06T19:19:20Z

packages/boilerplate-generator-tests/browser_test.js

+      url: '/packages/bootstrap/css/bootstrap-responsive.css?hash=785760fc5ad665d7b54d56a3c2522797bb2cc150&v="1"',
+      size: 22111,
+      hash: '785760fc5ad665d7b54d56a3c2522797bb2cc150' },
+   { path: 'packages/templating-runtime.js',


The indentation of this entry in the array should be the same as the one above it (one more space to the right).

abernix · 2017-07-06T19:20:06Z

packages/boilerplate-generator-tests/browser_test.js

+      cacheable: true,
+      url: '/packages/bootstrap/css/bootstrap-responsive.css?hash=785760fc5ad665d7b54d56a3c2522797bb2cc150&v="1"',
+      size: 22111,
+      hash: '785760fc5ad665d7b54d56a3c2522797bb2cc150' },


Let's align this closing brace with the opening brace.

abernix · 2017-07-06T19:20:22Z

packages/boilerplate-generator-tests/browser_test.js

+     cacheable: true,
+     url: '/packages/templating-runtime.js?hash=c18de19afda6e9f0db7faf3d4382a4c953cabe18&v="1"',
+     size: 24132,
+     hash: 'c18de19afda6e9f0db7faf3d4382a4c953cabe18' },


This brace should also be on its own line.

abernix · 2017-07-06T21:19:10Z

packages/boilerplate-generator/boilerplate_web_browser_template.js

+
+    _.map(js, ({url}) =>
+      _.template('  <script type="text/javascript" src="<%- src %>"></script>')({
+        src: bundledJsCssUrlRewriteHook(url) + '&hello=bye'


This hello=bye seems out of place. Is it meant to accomplish something?

ah, forgot to remove this from testing! thanks for catching it

abernix · 2017-07-06T21:34:33Z

packages/boilerplate-generator-tests/cordova_test.js

@@ -0,0 +1,90 @@
+import { parse, serialize } from 'parse5';
+
+function generateHTML() {


With the exception of the arch variable on the next line, it seems this is all duplicatd from the test in browser_test.js. We could DRY this up by making the generateHTML an import from a supporting library (for example, test-lib.js) that lives alongside the cordova_test.js and browser_test.js files, and supplying it with arch as a parameter.

abernix · 2017-07-06T22:23:37Z

packages/boilerplate-generator/boilerplate_web_cordova_template.js

+      // instead it should connect to 10.0.2.2
+      // (unless we\'re using an http proxy; then it works!)
+      '      if (!__meteor_runtime_config__.httpProxyPort) {',
+      '        __meteor_runtime_config__.ROOT_URL = (__meteor_runtime_config__.ROOT_URL || \'\').replace(/localhost/i, \'10.0.2.2\');',


I know some linting algorithms or style-guides may unnecessarily chastise this, but on the Meteor project, feel free to switch to double quotes to avoid the excessive escaping of single-quotes. (i.e. to avoid having to \'\')

abernix · 2017-07-06T22:23:49Z

packages/boilerplate-generator/boilerplate_web_cordova_template.js

+      '    if (/Android/i.test(navigator.userAgent)) {',
+      // When Android app is emulated, it cannot connect to localhost,
+      // instead it should connect to 10.0.2.2
+      // (unless we\'re using an http proxy; then it works!)


We don't need to escape the ' in this comment (it's not part of a string), but we should think about whether this comment was providing any value in the original template since it's no longer included now. (I think the value is low, so mainly just pointing this out).

abernix · 2017-07-06T22:43:03Z

packages/boilerplate-generator-tests/cordova_test.js

+
+Tinytest.add("boilerplate-generator-tests - web.cordova escape css", function (test) {
+  const html = generateHTML();
+  test.matches(html, /<link.*href=".*bootstrap.*&amp;v=&quot;1&quot;.*">/);


Not that it would happen, but just pointing out that this will also match:

<link rel="stylesheet" type="text/css" class="__meteor-css__" href="&v="1"bootstrap&v="1"">

Is that what you wanted? Maybe better to test for the absence of quotes and & or for an exact attribute value match? This might be easy to accomplish using parse5's parse?

abernix · 2017-07-06T22:44:07Z

packages/boilerplate-generator-tests/cordova_test.js

+  test.matches(html, /<link.*href=".*bootstrap.*&amp;v=&quot;1&quot;.*">/);
+});
+
+Tinytest.add("boilerplate-generator-tests - web.cordova do not call rewriteHook", function (test) {


Just informational, but if you add another - after web.cordova in these test names, they will be grouped into their own section in tinytest reports. :)

abernix · 2017-07-06T22:48:05Z

packages/boilerplate-generator-tests/browser_test.js

@@ -0,0 +1,100 @@
+import { parse, serialize } from 'parse5';


I think the name of this file should be consistent with that of the appropriate arch template (i.e. boilerplate_web.browser.template.js), and it should be plural like our other test files (e.g. web.browser-tests.js, web.cordova-tests.js)

Per the comment in meteor#8820 (comment) Previously, only the `constructor` method was addressed and this expands on that.

hwillson · 2017-07-07T12:51:47Z

This is looking great @stevenhao! One small thing to mention (and it's not necessarily something that should be considered with these changes, but I'm just mentioning it for discussion). With these changes we're still forcing the Meteor generated CSS to come before any externally referenced CDN based CSS files (in <head />). For the full backstory, see meteor/meteor-feature-requests#24 (and #8651). It's quite common practice to have 3rd party CDN based externally referenced CSS libraries come first in the CSS load order, which are then overridden as necessary by an application's CSS. Since we're refactoring the boilerplate it might be nice to also consider handling this scenario. Anyways, just food for thought - thanks for all of your work on this!

stevenhao · 2017-07-07T18:14:19Z

@hwillson, thanks for the comments! I agree that forcing the bundled css to come before custom css is not ideal, but I'm worried about backwards compatibility. Adding a configurable
flag to reorder stylesheets is a possible solution, but that may be out of the scope of this PR. What do you think @abernix?

@abernix

Letting @abernix take it from here

abernix · 2017-07-10T18:38:57Z

It's certainly worth mentioning the CSS load order here, but I think we should wrap this PR up and then iterate on it again as necessary.

This appears to be working properly in that when using static-html the server-side dependency on blaze is removed which was a goal of this PR (client was already removable). It's worth pointing out that there is still a dependency on templating-tools (in the Blaze repo) because of:

The CompileError class (via throwCompileError, here)
TemplatingTools.scanHtmlForTags.

I made a couple formatting changes but this is great work, @stevenhao!

Boilerplate refactor (meteor#8820)

stevenhao added 2 commits June 19, 2017 12:37

boilerplate refactor wip

cd50ba4

rename files

e4fdd29

stevenhao requested a review from glasser June 19, 2017 19:50

make switching between old/new easier

ad2601e

glasser reviewed Jun 19, 2017

View reviewed changes

hwillson mentioned this pull request Jun 21, 2017

Refactor boilerplate-generator package to remove server dependency on Spacebars/Blaze meteor/meteor-feature-requests#120

Open

benjamn added this to the Release 1.5.1 milestone Jun 21, 2017

abernix changed the title ~~[WIP] [Do not Merge] Boilerplate refactor~~ [Work in Progress] Boilerplate refactor Jun 21, 2017

stevenhao added 4 commits June 22, 2017 14:45

refactor and modernize boilerplate-generator

4365d86

add cordova template code

cedbf0f

delete old boilerplate-generator

f78f69a

small style fixes

4bfa9d1

benjamn reviewed Jun 22, 2017

View reviewed changes

address comments

5a66a8d

benjamn mentioned this pull request Jun 23, 2017

Server-side rendering revamp #8841

Merged

glasser previously requested changes Jun 24, 2017

View reviewed changes

benjamn pushed a commit that referenced this pull request Jun 26, 2017

Don't modify boilerplate.baseData when adding extraData.

9eb733f

This is a bug that will be fixed by @stevenhao's boilerplate-generator refactoring (#8820), but I need it fixed now :)

benjamn pushed a commit that referenced this pull request Jun 26, 2017

Don't modify boilerplate.baseData when adding extraData.

a01be34

This is a bug that will be fixed by @stevenhao's boilerplate-generator refactoring (#8820), but I need it fixed now :)

mitar mentioned this pull request Jun 27, 2017

SRI support #8847

Closed

stevenhao added 3 commits June 27, 2017 15:36

address review comments again

2a9d5f1

remove boilerplate generated-by comment

827c854

delete spacebars templates

c4e5fcc

stevenhao changed the title ~~[Work in Progress] Boilerplate refactor~~ Boilerplate refactor Jun 27, 2017

add boilerplate-generator-tests

bb85861

benjamn pushed a commit that referenced this pull request Jun 30, 2017

Don't modify boilerplate.baseData when adding extraData.

662f889

This is a bug that will be fixed by @stevenhao's boilerplate-generator refactoring (#8820), but I need it fixed now :)

stevenhao added 3 commits June 30, 2017 12:28

bump boilerplate-generator version

3371297

dummy commit

54fe867

Revert "dummy commit"

fc24b59

This reverts commit 54fe867.

update tests

59f85ed

stevenhao force-pushed the boilerplate-refactor-2 branch from 54a8ad1 to 59f85ed Compare July 5, 2017 19:27

stevenhao and others added 4 commits July 5, 2017 12:34

refactor parameter destructuring

b993d2f

fix style

40fb493

modernize boilerplate generator a bit

cd84309

Merge branch 'devel' into boilerplate-refactor-2

efad586

abernix suggested changes Jul 6, 2017

View reviewed changes

stevenhao added 4 commits July 6, 2017 17:03

refactor boilerplate-generator

aa77651

fix web browser template

b7e5596

refactor boilerplate-generator-tests

a3d57fb

rename files using hyphens

9d78ea6

abernix added 2 commits July 6, 2017 23:39

Remove spaces after object-shorthand method names.

e89a013

Per the comment in meteor#8820 (comment) Previously, only the `constructor` method was addressed and this expands on that.

Add some space for legibility between conditionals.

61b9f45

abernix approved these changes Jul 10, 2017

View reviewed changes

abernix modified the milestones: Release 1.5.2, Release 1.5.1 Jul 10, 2017

abernix merged commit 98b02f3 into meteor:devel Jul 10, 2017

GulajavaMinistudio mentioned this pull request Jul 10, 2017

Boilerplate refactor (#8820) GulajavaMinistudio/meteor#37

Merged

GulajavaMinistudio added a commit to GulajavaMinistudio/meteor that referenced this pull request Jul 10, 2017

Merge pull request #37 from meteor/devel

3270c8b

Boilerplate refactor (meteor#8820)

hwillson mentioned this pull request Jul 12, 2017

Cordova boilerplate does not play well with external scripts #8876

Closed

		@@ -11,11 +11,9 @@ Boilerplate = function (arch, manifest, options) {
		options = options \|\| {};

		@@ -0,0 +1,90 @@
		import { parse, serialize } from 'parse5';

		function generateHTML() {

Boilerplate refactor #8820

Boilerplate refactor #8820

Conversation

stevenhao commented Jun 19, 2017 • edited Loading

Summary of changes

apollo-cla commented Jun 19, 2017

Choose a reason for hiding this comment

benjamn Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjamn Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

stevenhao Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenhao Jun 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenhao Jun 26, 2017 • edited Loading

Choose a reason for hiding this comment

stevenhao Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenhao Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glasser commented Jun 27, 2017

abernix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwillson commented Jul 7, 2017

stevenhao commented Jul 7, 2017

abernix commented Jul 10, 2017

stevenhao commented Jun 19, 2017 •

edited

Loading

benjamn Jun 20, 2017 •

edited

Loading

benjamn Jun 20, 2017 •

edited

Loading

stevenhao Jun 21, 2017 •

edited

Loading

stevenhao Jun 26, 2017 •

edited

Loading

stevenhao Jun 26, 2017 •

edited

Loading

stevenhao Jun 27, 2017 •

edited

Loading

stevenhao Jun 27, 2017 •

edited

Loading