angular / router

The Angular 1 Component Router
MIT License
665 stars 135 forks source link

Problem with template convention for PascalCase component #210

Open bisubus opened 9 years ago

bisubus commented 9 years ago

There's a problem with mapping component name to template url. By default

function dashCase(str) {
  return str.replace(/([A-Z])/g, function ($1) {
    return '-' + $1.toLowerCase();
  });
}

causes PascalCase component to be mapped to ./components/-pascal-case/-pascal-case.html template, which is not supposed to be the correct behaviour (even if camelCase is preferable naming convention).

kamrenz commented 9 years ago

@bisubus What are you asking / proposing? It seems if we follow the convention of camelCase then there are no problems. If we don't follow the convention it breaks. What do you mean "not the correct behavior."

bisubus commented 9 years ago

@kamrenz The job of dashCase is to separate words with dashes but it fails to do it properly on 'PascalCase' string, I consider it a bug.

We use capitalised controller identifiers exclusively (great for namespacing and avoiding ng-repeat="product in product.products" mess), so it was hard to not spot erratic behaviour. Angular is rather opinionated but it shouldn't be an excuse for lazy coding, right?

I'm proposing something like this

function dashCase(str) {
  return str.replace(/(?!^)([A-Z])/g, '-$1').toLowerCase();
}

I doubt this is the real case for optimization, but it seems to be the fastest solution in general. Early IE versions were known for lookahead regex bugs, but this one is simple enough to work even there. Moreover, pre-IE9 issues won't cause problems for Angular.

jvandemo commented 9 years ago

@btford - If you feel this is a bug and should be supported, I can add this functionality and unit test(s) to PR #212.

It's a small fix so I don't see any harm in adding it, even though it's theoretically not supported by the convention.

We could also test for non-string values and return '' to avoid errors being thrown in case users pass a non-string value.

Just let me know what you think and I'll add the code and unit tests to the PR.