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

better helper checking #12

Closed
lifeart opened this issue Apr 18, 2019 · 12 comments
Closed

better helper checking #12

lifeart opened this issue Apr 18, 2019 · 12 comments

Comments

@lifeart
Copy link

lifeart commented Apr 18, 2019

https://github.com/rajasegar/ember-angle-brackets-codemod/blob/52a6b8ba9d4038aa55b95ff288052e9834a7ae77/transforms/angle-brackets/transforms/angle-brackets-syntax.js#L146

1.) we can suggest - all items without - - helpers
2.) items, having - in name and not having @, . in name - components
3.) items, having / in name - components

@lifeart
Copy link
Author

lifeart commented Apr 18, 2019

@lifeart
Copy link
Author

lifeart commented Apr 18, 2019

also, possible to get all project's components, using https://github.com/lifeart/ember-component-info (based on https://github.com/lifeart/ember-meta-explorer)

@rwjblue
Copy link
Member

rwjblue commented Apr 19, 2019

1.) we can suggest - all items without - - helpers

Hmm, this isn't true. Components haven't needed a - in their name for some versions (since at least 3.8 but possibly since initial angle bracket invocation support was added)

@lifeart
Copy link
Author

lifeart commented Apr 19, 2019

@rwjblue yes, but, it will cover 90% cases. Single-word components, after 3.8, with curly invocation - seems rare case

@jacobq
Copy link

jacobq commented Apr 19, 2019

2.) items, having - in name and not having @, . in name - components

This heuristic leads to the wrong conclusion for many helpers that I am currently using, many of which are in the helperNames you linked.

I'm not sure if this would make things easier or harder to implement, but my preference would for the codemod to:

  1. Allow a list of helper names ("blacklist") to be passed in, which it will assume are helpers, leave unchanged, and not make noise about.
  2. Allow a list of component names ("whitelist") to be passed in, which it should attempt to transform.
  3. Skip cases for which it is unable to tell for certain whether the expression refers to a helper or component invocation and then list those out at the end for the developer to work through manually or add to the aforementioned lists and re-run.

@GavinJoyce
Copy link
Collaborator

I'm experimenting with allowing a list of helpers to be provided here: https://github.com/rajasegar/ember-angle-brackets-codemod/pull/28

@GavinJoyce
Copy link
Collaborator

GavinJoyce commented Apr 24, 2019

https://github.com/rajasegar/ember-angle-brackets-codemod/pull/28 adds inital support for providing an optional config file. This currently supports providing a list of helpers in the following form:

{
  "helpers": [
    "ic-interface-icon", 
    "date-formatter"
  ]
}

@GavinJoyce
Copy link
Collaborator

@lifeart's ember-ast-hot-load should help get a list of helpers from a running app

@rajasegar
Copy link
Member

Thanks a lot @GavinJoyce for the help, much appreciated 🙏

@rajasegar
Copy link
Member

Can we close this issue or are we still missing something?

@patocallaghan
Copy link
Collaborator

@rajasegar I'd suggest we close it as we have the helpers config option now

@rajasegar
Copy link
Member

Thanks @patocallaghan

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

No branches or pull requests

6 participants