allow computed artifacts to request other computed artifacts #2018
Conversation
on the other hand, |
| if (this.cache.has(artifact)) { | ||
| return Promise.resolve(this.cache.get(artifact)); | ||
| if (this._cache.has(artifact)) { | ||
| return Promise.resolve(this._cache.get(artifact)); | ||
| } | ||
|
|
||
| return Promise.resolve().then(_ => this.compute_(artifact)).then(computedArtifact => { |
There was a problem hiding this comment.
don't you need to pass it in here?
There was a problem hiding this comment.
don't you need to pass it in here?
er, yeah, probably kind of important :)
|
PTAL |
| static instantiateComputedArtifacts() { | ||
| const computedArtifacts = {}; | ||
| require('fs').readdirSync(path.join(__dirname, 'computed')).forEach(function(file) { | ||
| // Drop `.js` suffix to keep browserify import happy. | ||
| file = file.replace(/\.js$/, ''); | ||
| const ArtifactClass = require('./computed/' + file); | ||
| const artifact = new ArtifactClass(); | ||
| const artifact = new ArtifactClass(computedArtifacts); |
There was a problem hiding this comment.
really not a fan of this intertwined computedArtifacts, artifact object, but I guess it works for now
There was a problem hiding this comment.
really not a fan of this intertwined computedArtifacts, artifact object, but I guess it works for now
sure, it uses inheritance for delegation, but on the plus side it guarantees invariants on our artifact dependency graph and it's nicely isolated, so it's ready for an easy refactor at any time :)
There's no reason that a computed artifact shouldn't be able to access other computed artifacts using the same artifact that it received. Since it can't access any new artifacts, just things based on the artifact it's already received, the cache will still work and computation of audits->requiredArtifacts->required gatherers still works.
should fulfill the needs of #2013
I've made a
ComputedArtifactstype for this object, and hadArtifactsextend it. Originally I was going to do that for the actual artifact object as well (make the computedArtifact object the prototype of the artifact object), but realized it wasn't necessary.