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

[feature request] Allow for subdirectories in components #1219

Closed
ssured opened this issue Jul 1, 2014 · 47 comments
Closed

[feature request] Allow for subdirectories in components #1219

ssured opened this issue Jul 1, 2014 · 47 comments

Comments

@ssured
Copy link

ssured commented Jul 1, 2014

I am developing a form editor, which currently consists of a multiple of components named like this:

/components/form-select.coffee
/components/form-input.coffee
/components/form-number.coffee
...

It would be nice if components (and their templates) can live in their own subdirectory, like this:

/components/form/select.coffee
/components/form/input.coffee
/components/form/number.coffee
...

To me it would be better for

  1. Project structure, in the file explorer it's easier to find what you are searching for
  2. Library development, it would allow for splitting the files of into a separate repo, having its own unit tests. The library can then easily be added using symlinks. Also this structure might benefit the plugin work @rjackson is doing.
@kimroen
Copy link
Contributor

kimroen commented Jul 1, 2014

Would you expect the components themselves to still be named form-select, form-input and form-number in this case? What is reasonable to expect?

@ssured
Copy link
Author

ssured commented Jul 1, 2014

The component names and usage can stay the same IMO. The resolver allows for replacing the file path /s into - for the components and the templates/components folder. Inside templates nothing changes and {{form-select}} should work, no matter if the component is stored in components/form-select.coffee or in components/form/select.coffee

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

You can generally allow for subdirectories in nearly all parts of the system (Routes, Controllers, Views, Templates, etc), but unfortunately due to a bug in Handlebars we cannot reference components (or helpers) in a template with a /. In theory, a custom resolver could be used to tweak the way components are looked up (like using a double dash to signify a slash or something), but I do not think that others will be OK with this (I actually suggested it during one of our core team meetings, and the others thought it to be too kludgy).

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

The Handlebars bug's fix: handlebars-lang/handlebars.js#797 (but that will only make it into 2.0 and since Ember does not support 2.0.0.alpha's yet we are stuck with no folders).

@ssured
Copy link
Author

ssured commented Jul 1, 2014

I am surprised this is a handlebars issue? We can tweak the resolver, right? Because components must have at least one -, IMO a change to the ember cli resolver should be fine...

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

Again, I suggested modifying the resolver to handle this, but that was decided to be a bad idea (Note: I still think it is a good one).

@ssured
Copy link
Author

ssured commented Jul 1, 2014

Judging from the contents of my own components folder:

bootstrap-progress-bar.coffee
bootstrap-progress.coffee
canvas-painter-leaflet.coffee
canvas-painter.coffee
em-nested-selectize.coffee
em-selectize.coffee
expanding-textarea.js
form-checkbox.coffee
form-field.coffee
form-form.coffee
form-input-number.coffee
form-input-text.coffee
form-input.coffee
form-list.coffee
form-select-nested.coffee
form-select.coffee
form-subform-select.coffee
form-subform.coffee
form.coffee
leaflet-layer-detail.coffee
leaflet-layer-geojson.coffee
leaflet-layer-mapquest.coffee
leaflet-layer-osm.coffee
leaflet-layer-tile.coffee
leaflet-layer-wms.coffee
leaflet-layer.coffee
leaflet-map.coffee
link-li.coffee
panel-tasks.coffee
pouch-state.coffee
st-image-div.coffee
st-image.coffee

I'd like to ask the core team to revisit the decision to allow for making subfolders in the components and templates/components folder. It just cleans up a lot of clutter. As far as I can tell adding this to the resolver won't conflict with handlebars-lang/handlebars.js#797, but I am not really familiar with that code.

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

I agree, and have submitted a PR: ember-cli/ember-resolver#56.

@stefanpenner
Copy link
Contributor

This deviation from the pattern makes me nervous as now dashes or slashes are ambiguous and if we ever add slashes they will mean something different

I would prefer to leave it how it is today

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

@stefanpenner - I understand. Could we make it -- or | to make it more palatable?

@stefanpenner
Copy link
Contributor

-- might work, I would like @wycats input on this.

TL;DR

having folders in the components directories would be nice for code-organization. This then does suffer from potential ambiguity or it suffers from inconsistency with the rest of the projects conventions.

If it was the case, that we could use / within the DOM or Handlebars aa {{foo/bar/baz }} or <foo/bar/baz></foo/bar/baz> I would feel instantly comfortable with adding this.

@wycats
Copy link

wycats commented Jul 1, 2014

Can someone explain to me what the -- syntax would look like?

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

@wycats - {{foo--bar}} -> app/components/foo/bar.js

@rwjblue
Copy link
Member

rwjblue commented Jul 1, 2014

We could also require a dash after the double-dash, so it would be:

{{foo--super-cool}} -> app/components/foo/super-cool.js

Essentially, the goal is to be able to organize component files on disk (in subfolders) and working around the Handlebars bug.

@stefanpenner
Copy link
Contributor

@wycats can you provide more input now that further clarification has been made?

@stefanpenner
Copy link
Contributor

this is a handlbars bug, will be fixed in htmlbars and apparently @tomdale is working on a backport. Unfortunately we wont support this feature until then.

@rwjblue
Copy link
Member

rwjblue commented Jul 4, 2014

It is fixed in Handlebars master (by @tomdale), but as far as I know will not be backported to Handlebars 1.x. I didn't realize he was working on backporting it further.

@ppcano
Copy link
Contributor

ppcano commented Aug 20, 2014

Registering a custom ComponentLookup supports the double-dash convention:

import ComponentLookup from 'ember-handlebars/component_lookup';

export default {
  name: 'component-lookup',
  after: 'registerComponentLookup',

  initialize: function(container, application) {

    var CustomComponentLookup = ComponentLookup.extend({
      lookupFactory: function(name, container) {
        name = name.replace('--', '/');
        return this._super(name, container);
      }
    });
    container.unregister('component-lookup:main');
    container.register('component-lookup:main', CustomComponentLookup);
  }
};

@rwjblue
Copy link
Member

rwjblue commented Aug 20, 2014

@ppcano - It would be easier to just implement an additional method to the resolver.

@ming-codes
Copy link

Now that we're on HTMLBars, is this still an issue?

@phcoliveira
Copy link

Maybe I am late to the discussion, since components can be nested in subdirectories and I am already using them this way, but I am missing the reason for imposing the dash in a component's name.

I thought that components would use their own names instead of classic tag names, and, for such, W3C recommends using a dash, like this: <my-tag></my-tag>, justifying the imposition. But, if this is true, how the slash is going to be handled, since it is not valid HTML markup? And, if this is not true, if components are not going to use their own names instead of classic tag names, then why imposing the dash in their names?

@stefanpenner
Copy link
Contributor

Maybe I am late to the discussion, since components can be nested in subdirectories and I am already using them this way, but I am missing the reason for imposing the dash in a component's name.

It actually boils down to us not believing / in a component name is appropriate. Our short term plan for pods, will likely enable "pod local" components and helpers (this prevents the long slashed pathed based component names), which means the dash will once again be required.

So removing the - requirement, becomes a hazard for ^ work.

@stefanpenner
Copy link
Contributor

Expect an updating pods RFC soon outlining this.

@jleppert
Copy link

This thread is everything about why I hate Ember.

@stefanpenner
Copy link
Contributor

@jleppert I appreciate the constructive feedback...

@ugoa
Copy link

ugoa commented Dec 12, 2015

@stefanpenner HAHAHAHAHAHA...

@jleppert
Copy link

I wasted over an hour of time trying to figure out why ember wasn't finding my component, giving no clue as to why. Only to find out that it's some random decision instead of letting people decide how to name their files...

@stefanpenner
Copy link
Contributor

@jleppert it is not random, it is to avoid ambiguity in the resolution of components and to prevent locking in functionality future work (local component lookup and some variants of angle bracket components) cannot support.

Now it would be handy if in development we detected these scenarios and warned/threw a helpful error. Unfortunately, your original comment failed to provide any actionable feedback, which prevented me from realizing the actual pain and recommending this as a potential resolution.

Although you may have frustrations, please be sure to provide it in the form of constructive feedback. Otherwise it is merely a wasted exchange. We respond quite well to polite constructive feedback, but aren't terribly motivated by excessively negative remarks (especially when they provide little or no concrete points).

@wycats
Copy link

wycats commented Dec 14, 2015

I wasted over an hour of time trying to figure out why ember wasn't finding my component, giving no clue as to why. Only to find out that it's some random decision instead of letting people decide how to name their files...

Now it would be handy if in development we detected these scenarios and warned/threw a helpful error. Unfortunately, your original comment failed to provide any actionable feedback, which prevented me from realizing the actual pain and recommending this as a potential resolution.

I agree with both of you. It seems like we have a painful error-case missing.

@rwjblue any interest in adding an explicit error in this scenario?

@jleppert
Copy link

I disagree. If your goal is to provide for convention over configuration, you think it's wise to break with the convention of a standard directory structure and naming convention that has been in use as the standard of almost every OS and file system? People expect that when you name a file something it should be made available as that name and be resolved as such. If you're concerned about ambiguity, you can use a name space like you've already done.

Imagine the frustration of creating a text file and naming it something only to find that the text editor not only won't open that file, it has no idea about the existence of said file because it doesn't conform to arbitrary naming conventions at the personal choice and whim of the text editor author? And not only that but it fails to tell the user the real problem, making an obvious problem more difficult and resulting in a discussion -- the file in fact was found, but we aren't considering it because it doesn't have the correct name.

@rwjblue
Copy link
Member

rwjblue commented Dec 14, 2015

@rwjblue any interest in adding an explicit error in this scenario?

I'm happy to add debugging aids (assertions/warnings/deprecations/etc) if an actual scenario is provided. All we've gotten so far is that @jleppert is frustrated, no steps to reproduce or explanation of what was happening.

@jleppert - Please open a new issue explaining the problems that you faced, and how we can reproduce them so that we might actually do something about this...

@wycats
Copy link

wycats commented Dec 14, 2015

I disagree.

@jleppert what exactly do you disagree with?

@stefanpenner
Copy link
Contributor

The resolution without a dash is ambiguous in this domain, as an angle bracket component resolved via local component lookup wouldn't be differentiable from a real DOM tag. As with web components, the dash ensures no conflict with current or future platform provided tags, and is intended to unambiguously refer to a custom tag.

This ultimately leads to two different mutually exclusive constraints, to resolve the tie we look to the domain where the entity is referenced html/htmlbars and consider the invariants (like DOM/web component naming conventions). It would be great if we could satisfy all constraints, but this scenario appears to require some trade offs.

@wycats we do currently provide an error if the component was created via generators, but it would appear we need some sort of additional pre-flight check. This may fall into the category of ember Doctor like scripts, or maybe at resolution time in dev mode all tags trigger a check via resolver for an invalid component name, erroring or warning. You are likely more familiar then I with this part of glimmer, do we have such a hook?

@wycats
Copy link

wycats commented Dec 16, 2015

Glimmer2 provides a one-time hook for "tag name" -> "component name" that we could use.

@brandonparsons
Copy link

In my opinion, a lot of this pain can also be fixed by putting components in a pod structure. I used to have a huge /components directory with all kinds of clutter, but found that by moving to pods (/pods/components/my-component/component.js & pods/components/my-component/template.hbs) it's all much cleaner.

Perhaps this option could be better advertised? I have been using Ember since pre 1.0 and didn't even know that this was an option until last week.

@stefanpenner
Copy link
Contributor

Glimmer2 provides a one-time hook for "tag name" -> "component name" that we could use.

Nice, that will be nice to hook into. Is this Glimmer or glimmer@2

In my opinion, a lot of this pain can also be fixed by putting components in a pod structure. I used to have a huge /components directory with all kinds of clutter, but found that by moving to pods (/pods/components/my-component/component.js & pods/components/my-component/template.hbs) it's all much cleaner.

Perhaps this option could be better advertised? I have been using Ember since pre 1.0 and didn't even know that this was an option until last week.

There are some things that must be cleaned up before we swap over to pods being the default. It is likely still a good choice, but until that work happens I am uneasy to make it the default. Someone needs to "own" that transition, and help move it forward.

@sergetab
Copy link

sergetab commented Jan 6, 2016

It actually boils down to us not believing / in a component name is appropriate. Our short term plan for >pods, will likely enable "pod local" components and helpers (this prevents the long slashed pathed >based component names), which means the dash will once again be required.

@stefanpenner Does that mean folders in components are not going to be supported in the future? We currently use them for our components.

@stefanpenner
Copy link
Contributor

@stefanpenner Does that mean folders in components are not going to be supported in the future? We currently use them for our components.

there will be support for components in folders, but it will likely be via the local component lookup pattern i believe @rwjblue is working on as part of some next steps in making pods a default pattern. I am unsure if their is an RFC with that or not

@kimroen
Copy link
Contributor

kimroen commented Jan 7, 2016

@sergetab Robert talked a bit about this in his talk at the Austin Ember Meetup, and again in the latest episode of EmberLand - maybe that would be of interest.

@runspired
Copy link
Contributor

I forget, but I believe there was a blocker @rwjblue faced in created an addon to test the "fractal pods" pattern. If so, has that cleared? And if so, want to spec it out a bit? I could take a stab at an addon implementing it to test with.

@lisp-ceo
Copy link

lisp-ceo commented Feb 1, 2016

👍 for "fractal pods"

@RustyToms
Copy link

If we could use the {{component}} helper in block form we would at least have a work around for this.

{{#component "directory/component-name"}}
  ...some stuff...
{{/component}}

@runspired
Copy link
Contributor

@RustyToms how does that relate to this?

@RustyToms
Copy link

Sorry, user error I think, it should work fine...

@egeexyz
Copy link

egeexyz commented May 12, 2016

It's unclear as it whether or not Ember supports component sub-directories. Some comments claim Ember supports it, while other comments do not. The official documentation does not say anything about sub-directories.

I am coming from the AngularJS world and my simple Ember project has 23 components. It seems like an oversight that sub-directories are not supported.

EDIT: StackOverflow answer - http://stackoverflow.com/questions/29038704/ember-components-in-subdirectory

@ibraheem4
Copy link

+1 would be a nice-to-have feature.

@kumkanillam
Copy link

kumkanillam commented Jul 20, 2017

@ibraheem4 @egee-irl you can create component in sub-directory. this will work as expected. While including you need to use / not . correct form of invocation is {{folder1/folder2/my-design }}

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