Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

Component templates in addon are not linted #292

Open
lstrzebinczyk opened this issue Jan 29, 2018 · 17 comments
Open

Component templates in addon are not linted #292

lstrzebinczyk opened this issue Jan 29, 2018 · 17 comments
Labels

Comments

@lstrzebinczyk
Copy link

lstrzebinczyk commented Jan 29, 2018

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?

@rwjblue
Copy link
Member

rwjblue commented Jan 29, 2018

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

@lstrzebinczyk
Copy link
Author

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?

@rwjblue
Copy link
Member

rwjblue commented Jan 30, 2018

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

@rwjblue
Copy link
Member

rwjblue commented Jan 30, 2018

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

Would definitely accept a PR upstream to fix...

@lstrzebinczyk
Copy link
Author

I'll give it a try.

@lstrzebinczyk
Copy link
Author

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
Copy link
Author

@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?

@Turbo87
Copy link
Member

Turbo87 commented Apr 4, 2018

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

@Turbo87 Turbo87 added the bug label Apr 4, 2018
@billybonks
Copy link

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

billybonks/ember-cli-stylelint#73.

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

@dfreeman
Copy link

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.

@dbashford
Copy link

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
Copy link

There anything that can be done locally?

For anyone who trips up on this.

ember-cli/ember-cli#7887 (comment)

⬆️ 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();
};

@rwjblue
Copy link
Member

rwjblue commented Sep 20, 2018

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

@dfreeman
Copy link

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.

@rwjblue
Copy link
Member

rwjblue commented Sep 20, 2018

@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 ember-cli/rfcs#121 makes progress...

@sangm
Copy link

sangm commented Sep 21, 2018

@dfreeman @rwjblue

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

@rwjblue
Copy link
Member

rwjblue commented Sep 22, 2018

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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants