-
Notifications
You must be signed in to change notification settings - Fork 2
Clean up Utils #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?
Clean up Utils #150
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| 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 } = {}) { | ||||||
|
|
@@ -43,18 +43,27 @@ exports.AssetManager = class AssetManager { | |||||
| }); | ||||||
| } | ||||||
|
|
||||||
| get packagesDir() { | ||||||
| let memo = this._packagesDir; | ||||||
| if(!memo) { | ||||||
| memo = this._packagesDir = this.resolvePath("./node_modules"); | ||||||
| } | ||||||
| return memo; | ||||||
| } | ||||||
|
|
||||||
| _updateManifest(originalPath, actualPath, targetDir) { | ||||||
| let { referenceDir } = this; | ||||||
| originalPath = path.relative(referenceDir, originalPath); | ||||||
| actualPath = path.relative(referenceDir, actualPath); | ||||||
| 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.
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.)