angular2-template-loader
angular2-template-loader copied to clipboard
templateUrl Regex fails to capture when comment follows
In the following legitimate code snippet:
@Component({
selector: "my-app",
templateUrl: './app.component.html'
//styleUrls: ["app.component.css"]
})
or this:
@Component({
selector: "my-app",
templateUrl: './app.component.html' //Very important comment
})
the regular expression templateUrlRegex
in index.js fails to capture, leaving the URL in the file to be treated as a separate html.
I can also confirm that comments are OK in 0.6.0 but not in 0.6.2
Version - "angular2-template-loader": "^0.6.2",
Probably this also has got related to the fact the templateUrl doesnt work without a comma . I found out this peculiar issue today. This does work ---
@Component({
selector: 'home',
templateUrl: "./home.template.html",
})
But This does not
@Component({
selector: 'home',
templateUrl: "./home.template.html"
})
The only difference is the comma after templateUrl: "./home.template.html"
Is also this issue related to #50 ?
I can confirm what @defyjoy posted. Just tried myself. Could the regex be updated?
I think my pull request #67 will fix this. If anyone else wants to try using that regex in their node_modules/angular2-template-loader/index.js
then just replace line 5 with:
var templateUrlRegex = /templateUrl\s*:(\s*['"`](.*?)([^\\]['"`])\s*)/gm;
If there's any cases it doesn't work in, I'd be happy to spend some time trying to make it work for those too.
@Riobe I swapped in your regex and it seems to have solved the issue. I only had the problem when templateUrl was the final member and was followed by a comment such as: `templateUrl: './ES-Building-EAL.component.html'
//styleurls: ...`
@charlie-elverson Glad it fixed your case. :)
I think I added a test case that matches that, because it seems like a common case (I hit it too), here
@Riobe Would it be worthwhile to also add a test case where the comment is on a new line? I'm terrible at regex, so I'm not sure if that requires a separate test case.
@charlie-elverson Looks like the common case is someone with a commented styleUrls so they take the comma out and it explodes. So I went ahead and added a test for that here. I think that would be functionally covered by the template last test, but it's better to be sure. :)
Of interest to me is that it actually changes the styleUrls property in the comment. I can't figure out how changing the output bundle's comment for that would cause an issue though, so I figure it's ok.
Is there any new reason for this. I see that @Riobe made a recommendation that exists in the current index.js file. Those are now the same. My file's see require and error out. The link on the docs for This is caused by the way the template loader works
Is broken.
I don't know what you mean new reason for this. My suggestion is in pull request #67 and I showed how you could hand modify this package to include it. @TheLarkInn hasn't replied to my PR yet so it's not in master and I don't know if it ever will be at this point since I created it in April.
My fix doesn't solve the issue the way he wanted, which was a more robust technique using an AST, but it would work for the cases I've seen, his previous tests, and the new ones I've added. If you want, you can fork the project and include it, but I have no idea if this project will get updated anymore.