allenhwkim / angular-jsdoc

AngularJS Template/Plugin for JSDoc 3.
MIT License
148 stars 57 forks source link

Template URL detection doesn't work correctly #72

Open reloaded opened 8 years ago

reloaded commented 8 years ago

The template URL matching doesn't work when the template URL is a concatenated string.

      templateUrl: "/client-src/app/directives/forms/equipment/" +
        "EquipmentManufacturersSelect.html",

The result of angular-jsdoc trying to parse the template URL above is:

http://localhost:8082/templates/_client-src_app_directives_forms_equipment_%20+

The problem is in the regular expression used to match the templateUrl string. I think it would be a better fool-proof alternative if a custom doc tag is used, something like @templateUrl/@templateurl.

I was experimenting with an improved regular expression here http://regexr.com/3d8ha . A custom doc tag will be more performant, easier for development maintenance and more flexible for different implementations/styles of code.

Zemke commented 8 years ago

Is this actually part of the ngDoc specification? I don’t think an @templateUrl tag exists and we shouldn’t invent our own custom tags.

How did you actually come about this? As far as I know template definitions of directives are not rendered into the documentation pages.

reloaded commented 8 years ago

The @templateUrl would be a custom doc tag used by angular-jsdoc to correctly locate the template URL. It may even be worth proposing to the Angular team to make that tag an official ngdoc tag.

I don't think there is anything wrong with angular-jsdoc defining it's own custom tags to achieve documentation functionality that is not provided by JSDOC2/3 and ngdoc out of the box. That's why JSDOC is flexible and supports custom tags.

Every Angular JS project is structured and designed differently so it will be impossible for angular-jsdoc to detect the templateUrl location string via a regular expression.

What if the string is on a single line? What if it's a string broken onto multiple lines via concat to enforce an 80 character line length code style? What if the directive injects a custom config service and uses that service to get the template URL (e.g. $configService.templateUrls.myDirective).

What are your thoughts?

Zemke commented 8 years ago

I don’t see a point in adding the path pointing to the template file to the documentation. What does it actually contribute to the documentation of the directive? I think there’s a point in not having a tag for that.

reloaded commented 8 years ago

The reason for a custom @templateUrl tag is to tell angular-jsdoc where it can locate (on the file system) the HTML template file.

If you look at the sample code below you will see that the templateUrl string is on one line. https://github.com/allenhwkim/angular-jsdoc/blob/master/sample-codes/ngmap/map.js#L64

If you break that string to multiple lines using concatenation angular-jsdoc will no longer locate the template on the file system.

For example if you change that line to

templateUrl: 'sample-codes/ngmap/' +
   test-template.html'

angular-jsdoc will think the template path is literally "sample-codes/ngmap/ +". It doesn't work on multi-line template URL strings which means it also won't work in the scenario where someone uses an Angular service/factory or javascript variable to define where the template is in the project file system.

reloaded commented 8 years ago

The custom tag using the sample code file above would be:

@templateUrl sample-codes/ngmap/test-template.html
reloaded commented 8 years ago

Here's a real example of the problem I have run into.

image

My directive template consists of a JS variable concatenated with a string that contains the HTML file name. angular-jsdoc can't parse the content of my JS variable which is named baseTemplateUrl so the angular-jsdoc thinks my template URL is baseTemplateUrl%20+%20EquipmentStatusSelect.html.

Does all of this make sense or am I not explaining it well enough?

Zemke commented 8 years ago

Thanks for the elaboration. I get what you mean. I’ve just noticed that directive templates were part of the generated documentation. I’ve actually never noticed that there’s this template link and the template source was shown there. 🙃

Well, I wouldn’t mind adding this, but we’d be breaking existing functionality. Maybe can we find a backward-compatible solution?

reloaded commented 8 years ago

@Zemke We don't have to break the current implementation at this time. The backward compatibility style would be:

1) Look for @templateUrl in the directive doc block and use it if it's there. 2) If it's not there, and if the regex match is true (meaning the directive has the templateUrl object property) console.log a "deprecated" notice and use the regex extracted value. 3) In the next major version, remove the template regex. all together.

allenhwkim commented 8 years ago

It was my mistake to read the source code to generate document especially for the template.

https://github.com/allenhwkim/angular-jsdoc/blob/0551a993146657d0f40f658e9160f5971de3eb45/angular-template/publish.js#L214

The benefit of having templateUrl is that we can read template from the generated documentation.

I thought it was convenient to read the template of a directive from source code rather than a tag, but it causes some issues. expression, variable, etc.

I think we should start using tag instead.

krgowtham commented 7 years ago

Hello, Any update on this issue? When will the @templateUrl tag be available?