Cache Project model config. #7270
Conversation
lib/models/builder.js
Outdated
| @@ -171,7 +171,7 @@ class Builder extends CoreObject { | |||
| .catch(error => { | |||
| this.processAddonBuildSteps('buildError', error); | |||
| throw error; | |||
| }); | |||
| }).finally(() => this.project.configCache.clear()); | |||
There was a problem hiding this comment.
Minor nit-pick, the rest of this promise chain is dot aligned, I would expect this .finally to be on the next line.
| @@ -52,6 +52,7 @@ class Project { | |||
| this.setupNodeModulesPath(); | |||
| this.addonDiscovery = new AddonDiscovery(this.ui); | |||
| this.addonsFactory = new AddonsFactory(this, this); | |||
| this.configCache = new Map(); | |||
There was a problem hiding this comment.
Can you make this _configCache? We don't want it to be considered public API on Project.
lib/models/project.js
Outdated
| * @param {String} env Environment name | ||
| * @return {Object} Merged confiration object | ||
| */ | ||
| configWithoutCache(env) { |
There was a problem hiding this comment.
I doc'd it as private is that not enough?
There was a problem hiding this comment.
there is a lot of @private methods without underscore.
krisselden
force-pushed
the
project-config-cache
branch
from
47c9b85
to
ef212da
Compare
|
@krisselden and I tracked down the failure to an issue with an addon mutating the provided |
Addons have the expectation that project.config is "cheap" to call, and this is not true. The config for a build should be stable.
rwjblue
force-pushed
the
project-config-cache
branch
from
ef212da
to
09eaef2
Compare
|
Updates:
|
|
Ugh, CI is now failing due to a bad merge conflict resolution I did when merging |
|
#7276 was fixed, restarting Travis now... |
|
This change has had an undesired effect on our code which I just noticed while trying to update We may have been abusing the We want to change the contents of our addon's merged config dependent on a setting from the consuming app. We were achieving this by setting a variable in the addons The first time Should a workflow like this be supported? Is there any way for the The reason that we want to do this at compile-time rather than run-time is to redact certain pieces of config on publicly visible apps, otherwise we could just filter at runtime when all of the variables are available... |
Add-ons have the expectation that project.config is "cheap" to call and this is not true. The config for a build should be stable.