Skip to content
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

Cannot disable hinting in addon when isDevelopingAddon is true #5594

Closed
carlbennettnz opened this issue Mar 7, 2016 · 5 comments
Closed

Comments

@carlbennettnz
Copy link

I have an app which includes an npm linked addon in its package.json. Both the app and the addon have hinting: false in their ember-cli-build.js files, and the addon has isDevelopingAddon: () => true in its index.js. If I ember serve the app, the hinting option is ignored for the addon.

@Turbo87
Copy link
Member

Turbo87 commented Apr 13, 2016

@carlbennettnz I'm not sure if this is a scenario that we support. Does it work properly when using ember serve from the addon folder?

@carlbennettnz
Copy link
Author

Yes.

  • ember serve from addon folder
  • ember serve from app folder, with isDevelopingAddon: () => false
  • ember serve from app folder, with isDevelopingAddon: () => true

In the final case, everything seems functionally correct, but the JSHint errors are shown.

@rwjblue
Copy link
Member

rwjblue commented Apr 13, 2016

https://github.com/ember-cli/ember-cli/blob/master/lib/models/addon.js#L119-L124 should be updated something like:

Addon.prototype.hintingEnabled = function() {
  var isProduction = process.env.EMBER_ENV === 'production';
  var testsEnabledDefault = process.env.EMBER_CLI_TEST_COMMAND || !isProduction;
  var hintingDisabled = this.options && this.options.hinting === false;

  return testsEnabledDefault && !hintingDisabled;
};

@Turbo87
Copy link
Member

Turbo87 commented Apr 13, 2016

@carlbennettnz could you check if the solution proposed by @rwjblue works for you? if it does feel free to open a PR :shipit:

@rwjblue
Copy link
Member

rwjblue commented Apr 13, 2016

(with tests)

jeffjewiss added a commit to jeffjewiss/ember-cli that referenced this issue Apr 19, 2016
Closes ember-cli#5594

* checks for the app options hinting to be set to false
* adds tests for the hintingEnabled method
jeffjewiss added a commit to jeffjewiss/ember-cli that referenced this issue Apr 24, 2016
Closes ember-cli#5594

* checks for the app options hinting to be set to false
* adds tests for the hintingEnabled method
homu added a commit that referenced this issue Apr 24, 2016
Fix for disabling hinting for developing an addon

Please let me know if these aren't the types of tests you're looking for to cover this scenario. I figured it was worth trying to write them to get some feedback. Thanks!

Closes #5594
trentmwillis pushed a commit to trentmwillis/ember-cli that referenced this issue May 5, 2016
Closes ember-cli#5594

* checks for the app options hinting to be set to false
* adds tests for the hintingEnabled method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants