- 
        Couldn't load subscription status. 
- Fork 2
v3.1.0 #150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
v3.1.0 #150
Changes from 2 commits
17dfc2f
              93d35ec
              82a7635
              1a40eb5
              b8ad295
              c9c4c9f
              85af017
              73777b8
              7bf71e2
              3bf9b98
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|  | @@ -2,9 +2,9 @@ | |||||
|  | ||||||
| let { Manifest } = require("./manifest"); | ||||||
| let { createFile } = require("./util/files"); | ||||||
| let { resolvePath } = require("./util/resolve"); | ||||||
| let { reportFileStatus, abort, generateFingerprint } = require("./util"); | ||||||
| let { reportFileStatus, abort, resolvePath } = require("./util"); | ||||||
| let path = require("path"); | ||||||
| let crypto = require("crypto"); | ||||||
|  | ||||||
| exports.AssetManager = class AssetManager { | ||||||
| constructor(referenceDir, { manifestConfig, fingerprint, exitOnError } = {}) { | ||||||
|  | @@ -58,3 +58,20 @@ exports.AssetManager = class AssetManager { | |||||
| return this.manifest.set(originalPath, actualPath, targetDir); | ||||||
| } | ||||||
| }; | ||||||
|  | ||||||
| function generateFingerprint(filepath, data) { | ||||||
| let filename = path.basename(filepath); | ||||||
| let ext = filename.indexOf(".") === -1 ? "" : "." + filename.split(".").pop(); | ||||||
| let name = ext.length === 0 ? filename : path.basename(filepath, ext); | ||||||
| let hash = generateHash(data); | ||||||
| return path.join(path.dirname(filepath), `${name}-${hash}${ext}`); | ||||||
| } | ||||||
|  | ||||||
| // exported for testing | ||||||
| exports.generateFingerprint = generateFingerprint; | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 That makes it explicit; cf.  | ||||||
|  | ||||||
| function generateHash(str) { | ||||||
| let hash = crypto.createHash("md5"); | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we perhaps change this to SHA-256? I realize that might be a backwards-compatibility concern though... 
 | ||||||
| hash.update(str); | ||||||
| return hash.digest("hex"); | ||||||
| } | ||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,7 @@ | ||||||||||
| "use strict"; | ||||||||||
|  | ||||||||||
| let fs = require("fs"); | ||||||||||
| let path = require("path"); | ||||||||||
| 
      Comment on lines
    
      +3
     to 
      4
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might as well do this while we're at it: 
        Suggested change
       
 | ||||||||||
| let crypto = require("crypto"); | ||||||||||
|  | ||||||||||
| exports.abort = abort; | ||||||||||
| exports.repr = repr; | ||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, I had intentionally moved exports to the top (yay hoisting) so you don't have to go searching for them in the middle of random implementation details. 🤷 | ||||||||||
|  | ||||||||||
| // reports success or failure for a given file path (typically regarding | ||||||||||
| // compilation or write operations) | ||||||||||
|  | @@ -26,28 +23,39 @@ exports.loadExtension = async (pkg, errorMessage, supplier = pkg) => { | |||||||||
| } | ||||||||||
| }; | ||||||||||
|  | ||||||||||
| exports.generateFingerprint = (filepath, data) => { | ||||||||||
| let filename = path.basename(filepath); | ||||||||||
| let ext = filename.indexOf(".") === -1 ? "" : "." + filename.split(".").pop(); | ||||||||||
| let name = ext.length === 0 ? filename : path.basename(filepath, ext); | ||||||||||
| let hash = generateHash(data); | ||||||||||
| return path.join(path.dirname(filepath), `${name}-${hash}${ext}`); | ||||||||||
| }; | ||||||||||
|  | ||||||||||
| function abort(msg, code = 1) { | ||||||||||
| console.error(msg); | ||||||||||
| process.exit(code); | ||||||||||
| } | ||||||||||
| exports.abort = abort; | ||||||||||
|  | ||||||||||
| function repr(value, jsonify = true) { | ||||||||||
| if(jsonify) { | ||||||||||
| value = JSON.stringify(value); | ||||||||||
| } | ||||||||||
| return `\`${value}\``; | ||||||||||
| } | ||||||||||
| exports.repr = repr; | ||||||||||
|  | ||||||||||
| function generateHash(str) { | ||||||||||
| let hash = crypto.createHash("md5"); | ||||||||||
| hash.update(str); | ||||||||||
| return hash.digest("hex"); | ||||||||||
| } | ||||||||||
| exports.resolvePath = (filepath, referenceDir, { enforceRelative } = {}) => { | ||||||||||
| if(/^\.?\.\//.test(filepath)) { // starts with `./` or `../` | ||||||||||
| return path.resolve(referenceDir, filepath); | ||||||||||
| } else if(enforceRelative) { | ||||||||||
| abort(`ERROR: path must be relative: ${repr(filepath)}`); | ||||||||||
| } else { // attempt via Node resolution algorithm | ||||||||||
| try { | ||||||||||
| return require.resolve(filepath, { paths: [referenceDir] }); | ||||||||||
| } catch(err) { | ||||||||||
| // attempt to resolve non-JavaScript package references by relying | ||||||||||
| // on typical package paths (simplistic approximation of Node's | ||||||||||
| // resolution algorithm) | ||||||||||
| let resolved = path.resolve(referenceDir, "node_modules", filepath); | ||||||||||
| try { | ||||||||||
| fs.statSync(resolved); // ensures file/directory exists | ||||||||||
| } catch(_err) { | ||||||||||
| abort(`ERROR: could not resolve ${repr(filepath)}`); | ||||||||||
| } | ||||||||||
| return resolved; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| }; | ||||||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -21,6 +21,10 @@ let DEFAULTS = { | |
| bucket: "static", | ||
| plugin: "faucet-pipeline-static" | ||
| }, | ||
| assets: { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this file is also the place where we would mark the old name as deprecated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not currently sure how exactly we'd do that, especially because plugins can override these defaults? I guess we'll have to put on our hazmat suits and wade into the implementation eventually... | ||
| bucket: "static", | ||
| plugin: "faucet-pipeline-static" | ||
| }, | ||
| images: { | ||
| bucket: "static", | ||
| plugin: "faucet-pipeline-images" | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we not using
path.extnamehere? (See potentially related discussion forfaucet-static/static-images.)(Though we might reasonably opt not to risk changing the implementation here, due to backwards-compatibility concerns.)