ember-cli-template-lint icon indicating copy to clipboard operation
ember-cli-template-lint copied to clipboard

Component templates in addon are not linted

Open lstrzebinczyk opened this issue 7 years ago • 17 comments

We would like to lint templates of components in an addon, but they don't seem to be linted. ESLint is not linting in addons too, unless one specifies

isDevelopingAddon() {
  return true;
}

in index.js, which we do. Is there any way of enabling this feature?

lstrzebinczyk avatar Jan 29 '18 14:01 lstrzebinczyk

I don't understand the question? If an addon includes isDevelopingAddon() { return true; } its templates should naturally be linted (on current ember-cli versions).

rwjblue avatar Jan 29 '18 21:01 rwjblue

Templates inside /addon are linted, while templates inside /app are not. I created a reproduction repo to show this: https://github.com/KillaPL/template-lint-issue

Am I incorrect to expect templates inside /app to be linted, or is this a bug?

lstrzebinczyk avatar Jan 30 '18 11:01 lstrzebinczyk

Ya, it was a bug, but it was fixed on newer Ember-cli versions I believe. What version are you using?

rwjblue avatar Jan 30 '18 13:01 rwjblue

Hmm, I was thinking of https://github.com/ember-cli/ember-cli/pull/7119 (and https://github.com/ember-cli/ember-cli/pull/7369), but I don’t think I fixed templates in an addons app there...

Would definitely accept a PR upstream to fix...

rwjblue avatar Jan 30 '18 13:01 rwjblue

I'll give it a try.

lstrzebinczyk avatar Jan 30 '18 15:01 lstrzebinczyk

Just to keep you informed: I started working on this and already have a broad idea of how this should be done, but I had to focus on some other work related issues. I'm still on it.

lstrzebinczyk avatar Feb 07 '18 12:02 lstrzebinczyk

@rwjblue I seem to be stuck on it, there might be a bigger issue to solve. Or maybe I'm just not quite understanding what's going on here.

So as I understand it, I must add a proper funnel to jshintAddonTree method here: https://github.com/ember-cli/ember-cli/blob/master/lib/models/addon.js#L1094

I am able to do that by adding this code to method:

let appTemplatesPath = this._treePathFor('templates');
if (existsSync(appTemplatesPath)) {
  let appTree = new Funnel(appTemplatesPath, {
    destDir: '/app/templates',
  });
  let lintAppTemplatesTrees = this._eachProjectAddonInvoke('lintTree', ['templates', appTree]);
  trees = trees.concat(lintAppTemplatesTrees);
}

But this seems to override previous adding of templates from the addon directory.

I tried adding those templates in multiple ways, but I can always only have the templates from either app or addon directories added, never both at the same time.

I also tried returning all of them from _addonTemplateFiles, which is doable, but then all templates are presented in tests report as if they came from addon directory.

I suspect that this might come from this line:

let lintAppTemplatesTrees = this._eachProjectAddonInvoke('lintTree', ['templates', appTree]);

overriding this:

let lintTemplateTrees = this._eachProjectAddonInvoke('lintTree', ['templates', addonTemplates]);

from here: https://github.com/ember-cli/ember-cli/blob/master/lib/models/addon.js#L1113

Or, more specifically, the last Funnell tree concatenated into the return value gets accepted as the tree with the templates, overriding previous ones. Even when they have different destDir.

This is where I'm stuck, and would love to hear some feedback. Am I missing something obvious and simple, or is there a deeper issue to solve?

lstrzebinczyk avatar Feb 07 '18 17:02 lstrzebinczyk

@billybonks I think you touched some of that code lately. Any idea about this?

Turbo87 avatar Apr 04 '18 16:04 Turbo87

i do have a few ideas, let me explore the issues identified here and get back to you.

btw i resolve all these issues with

https://github.com/billybonks/ember-cli-stylelint/issues/73.

i pretend that lintTree only ever gets called once and fetch everything that i want :), or that the user configures.

billybonks avatar Apr 05 '18 16:04 billybonks

I would guess more people are going to start running into this now that template-lint is part of the default addon blueprint.

The lint:hbs script catches things in addon, since it's just scanning the project directory, but a few folks across different teams here have been tripped up by the generated tests only including the dummy app.

dfreeman avatar Sep 12 '18 18:09 dfreeman

We have route templates in /templates. Those and the dummy app are the only things being linted. Everything in /components is being missed. There anything that can be done locally?

dbashford avatar Sep 20 '18 15:09 dbashford

There anything that can be done locally?

For anyone who trips up on this.

https://github.com/ember-cli/ember-cli/issues/7887#issuecomment-400663710

⬆️ That seems to work for me.

const { WatchedDir } = require('broccoli-source');
const templatesPath = `${__dirname}/addon/components`;
const templatesTree = new WatchedDir(templatesPath);

module.exports = function(defaults) {
  const app = new EmberAddon(defaults, {
    trees: {
      templates: templatesTree
    }
  });
  return app.toTree();
};

dbashford avatar Sep 20 '18 17:09 dbashford

TBH, I'd suggest using yarn lint:hbs as the primary mechanism to do the linting tests...

rwjblue avatar Sep 20 '18 19:09 rwjblue

Totally on board with lint:hbs as a better overall solution, but I feel like it sets confusing expectations to generate lint tests for some things and not others, particularly given that ember-cli-eslint is most people's point of reference, and it does work on code in addon.

dfreeman avatar Sep 20 '18 20:09 dfreeman

@dfreeman I think I am lumping ember-cli-template-lint into the same category as ember-cli-eslint, and am assuming it will be removed from the blueprint in favor of lint:hbs once https://github.com/ember-cli/rfcs/pull/121 makes progress...

rwjblue avatar Sep 20 '18 20:09 rwjblue

@dfreeman @rwjblue

I assume we will leave the scope of the RFC as it is?

sangm avatar Sep 21 '18 00:09 sangm

Sorry @sangm, I wasn’t suggesting that the RFC change at all, just that this library is likely to follow whatever the decision that is made there (it is nearly the same exact situation).

rwjblue avatar Sep 22 '18 13:09 rwjblue