vigetlabs / blendid

A delicious blend of gulp tasks combined into a configurable asset pipeline and static site builder
MIT License
4.97k stars 683 forks source link

http2 upgrade #495

Closed benjtinsley closed 6 years ago

benjtinsley commented 6 years ago

big picture

this adds a command to run after yarn run blendid -- init so that developers can take advantage of HTTP/2 multiplexing

this only affects the html, javascript and stylesheets directories within src and currently only works as expected with the default init task, though will most likely work with Drupal and Craft init tasks as well.

to test it yourself

  1. pull this branch down
  2. run the standard yarn install command
  3. copy the extras/http2 directory and paste it into node_modules/blendid/extras
  4. copy the gulpfile.js/tasks/http2-upgrade.js file and paste it into node_modules/blendid/gulpfile.js/tasks`
  5. run yarn run blendid -- init
  6. run yarn run blendid -- http2-upgrade
  7. run yarn run blendid and you should see this page:
screen shot 2017-09-13 at 4 08 48 pm

todo before merging

olets commented 6 years ago

Tangential question (cc @nhunzaker): is there a storage, performance, or other processing-related reason to choose between const, let, and var in cases where more than one would work from a logic and scope standpoint?

Example 1: backdaday, the required things in a gulp file were vars. Indeed a lot of Blendid's gulp files still pull dependencies in as vars. Is a const preferable?

Example 2: I haven't spent enough time with JS to have a strong personal style, but I think I'd lean towards using let in narrow-scope situations (e.g. configStream - http2-upgrade.js:15) even if it isn't going to be reassigned

To be clear: Ben what you did fits with the rest of Blendid, I'm not recommending you mess around with your uses of const

benjtinsley commented 6 years ago

@olets thanks for the review. from my understanding, const sets a variable under the assumption that it cannot be reassigned, allocating less memory to watching for a change. let explicitly says the value will be reassigned, so it needs some extra memory in order to be sure that its reassignment is captured. var is just the older style catchall, that says it could or could not change, who knows? i think the memory allocated for var is similar to let.

in summation, const should be the default unless you know that it will be reassigned, in which case use let.

olets commented 6 years ago

Re: let: it says it can be reassigned, but also that it's only going to be used in the block it's defined in. So as far as those two aspects go, seems like there's some grey area — you could choose to use const to signal the processor that it's not going to be reassigned, or choose to use let to signal the human reader that its scope is limited. Since a const's properties can change value some memory must be used to watch for those changes.

Anyway, I should take this to Slack to try to learn more about it all

greypants commented 6 years ago

Re: let vs const. This project was born before const was a thing, and we were still supporting node 0.12.x. We've dropped support for old node, so feel free to replace all those vars.

benjtinsley commented 6 years ago

a follow up to your concern @olets:

To me the example stylesheet partials feel heavy for something users will almost certainly immediately delete.

i think this is a valid point but i believe the initial state enforces the readme, just because setup in this update is fairly different. not only am i telling people how it should be done, but i'm showing them as well. i know that people will delete these files and directories, but i put more emphasis on making it easy to understand for a first time user than making it a minor bit time consuming for a return user. if a first time user doesn't get it or uses it incorrectly, then there is no benefit to this feature for them, and they will either stop using this or use it incorrectly over and over.

i'd like for people to understand the readme and i do want to make it as clear as possible, but i also know people pull things down and try to start working with it after sorta skimming over it, so i think in this case an example is necessary so that it shows users the intended use case.

olets commented 6 years ago

Fair enough, and it's true this fits with the rest of the demo files