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

Clean up moduleId, add ability to specify ignore via file path with extension #2515

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

jamescdavis
Copy link
Contributor

@jamescdavis jamescdavis commented May 19, 2022

As described in #2128, when ignore is configured, moduleId is still required when calling the node API's verify() despite the docs making no mention of this. This remedies that by cleaning up usage of moduleId internally as proposed in #2128.

There is at least one user of the node API that this is affecting: mansona/lint-to-the-future-ember-template#12

The one place I've left any reference to moduleId is within lib/-private/module-status-cache because configuration of ignore for specific files (non-glob) currently requires specification as a module instead of a file path with extension, e.g.

// .template-lintrc.js
module.exports = {
  ignore: ['path/to/template'],
};

I have, though, added the ability to also specify specific files via path with extension, e.g.

// .template-lintrc.js
module.exports = {
  ignore: ['path/to/template.hbs'],
};

and updated the docs to use this instead of by module.

This was motivated in part with wanting to have consistency with overrides (see #2513).

We may want to deprecate specifying by module and remove in next major version.

@jamescdavis
Copy link
Contributor Author

jamescdavis commented May 19, 2022

This is out of scope for this PR, but should we update this example

ignore: [
'project-name/templates/login',
'project-name/templates/components/odd-ones/**',
'app/templates/login',
'app/templates/components/odd-ones/**',
]

to no longer duplicate ignores and remove

## Why patterns are duplicated
You may have noticed in the sample configuration that the four patterns are actually two patterns repeated with different prefixes.
This is because `ember-template-lint` works directly with the filesystem and is not aware
of the applications module prefix, however `ember-cli-template-lint` works within the build
pipeline of your application which is not aware of the original file paths since all modules
are within your applications module prefix by the time it is ran.
**_tldr;_** The `app/...` patterns match the filesystem (app directory). The `project-name/...` patterns match Ember's resolver (`ENV.modulePrefix` in `config/environment.js`).

now that ember-cli-template-lint is deprecated?

@jamescdavis
Copy link
Contributor Author

Also

After [Module Unification](https://github.com/emberjs/ember.js/issues/16373), Ember's internal resolver will match the filesystem, so ignore patterns will no longer need to be repeated.

should probably be removed at this point, regardless.

@MelSumner MelSumner requested review from bmish and rwjblue August 3, 2022 19:26
@bmish
Copy link
Member

bmish commented Aug 3, 2022

@jamescdavis very sorry about the delay on this. A few questions:

  1. Can you rebase this on the latest master?
  2. Can you explain in a bit more detail if this is an internal-only change, a bug fix, a feature, or a breaking change?
  3. Is this ready to merge?

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.

None yet

2 participants