Skip to content

Fix using the addon as a runtime dependency of another addon#43

Merged
jneurock merged 1 commit intokloeckner-i:masterfrom
bertdeblock:fix-using-addon-as-runtime-dependency
Apr 13, 2020
Merged

Fix using the addon as a runtime dependency of another addon#43
jneurock merged 1 commit intokloeckner-i:masterfrom
bertdeblock:fix-using-addon-as-runtime-dependency

Conversation

@bertdeblock
Copy link
Copy Markdown
Contributor

@bertdeblock bertdeblock commented Apr 13, 2020

When including this addon as a runtime dependency of another addon, I encountered the following error: Cannot read property 'project' of undefined.

The error occurs at this line: https://github.com/kloeckner-i/ember-cli-mirage-graphql/blob/master/index.js#L25

I have set up a reproduction here: https://github.com/bertdeblock/ember-cli-mirage-graphql-repro
The error should occur if you run ember s (so running the dummy app).

This PR should fix this error. 🤞

Let me know if you want me to change anything.

Comment thread index.js
},
included(app) {
let config = this.app.project.config(app.env);
let config = app.project.config();
Copy link
Copy Markdown
Contributor Author

@bertdeblock bertdeblock Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using app instead of this.app is the actual fix for the error described in the PR description.

Comment thread index.js
let config = app.project.config();

this.appEnv = app.env;
this.appEnv = config.environment;
Copy link
Copy Markdown
Contributor Author

@bertdeblock bertdeblock Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When included as a sub-dependency, app.env seems to be undefined which makes the addon exclude itself from the build because of this check. Using config.environment seems to fix this. Not sure if this is the "correct" fix though.

@jneurock
Copy link
Copy Markdown
Contributor

Thanks for the PR! Since it's late, I'll release this tomorrow morning.

@jneurock jneurock merged commit 62d46fc into kloeckner-i:master Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants