inline templates and styles needs regex improvement

jvandemo / generator-angular2-library

Yeoman generator to create an Angular library

MIT License

752 stars 122 forks source link

inline templates and styles needs regex improvement #79

Open BioPhoton opened 7 years ago

BioPhoton commented 7 years ago

I notices some wrong behavior when trying to use the new inline features of v9.0.0.

After research i found following things:

the regex for tempalteUrl and styleUrls works only with ' not with "
the regex for templateUrl and styleUrls parses also the comments and strings. It should only parse the related property

here is a test that shows the testcases: https://regex101.com/r/M6pB9M/2

caroso1222 commented 7 years ago

Hey @BioPhoton nice catch thanks! I think the edge cases you're proposing are all valid. I took a stab at fixing this, take a look here: https://regex101.com/r/M6pB9M/4. Can you please double check we're not missing any edge case here?

BioPhoton commented 7 years ago

Hi @caroso1222! I added a view more checks with comments if they are imho not right... https://regex101.com/r/M6pB9M/6

jvandemo commented 7 years ago

@BioPhoton — Nice catch, thank you!

Would you be interested in creating a PR to fix this?

I don't think we should worry about the double quotes., because the tslint.json is configured to only accept single quotes.

What are your thoughts? Thanks!

franzeal commented 7 years ago

Just passing by and thought I'd try some tweaks and add another test case: https://regex101.com/r/vIRDKR/5 https://regex101.com/r/vIRDKR/6

Not sure how to deal with comments without lookbehind.

jvandemo commented 7 years ago

@franzeal — Thank you for the additional tests, much appreciated!

jvandemo commented 7 years ago

Quick update: this regex is used in the Angular SystemJS loader: /templateUrl\s*:(\s*['"](.?)['"`]\s)/gm`

https://github.com/angular/angular/blob/061475402c26759a2d7c0840b99507320c3dffc4/aio/tools/examples/shared/boilerplate/src/systemjs-angular-loader.js#L1

It looks like this could work if we could first filter out comments.

What are your thoughts? Thanks!

e-cloud commented 7 years ago

I think this is a bug but not enhancement. Discussion values but takes too long. A beta version may be better. @BioPhoton do you have time to make it or I will give it a try.

BioPhoton commented 7 years ago

Yes I think the same...

I already added more rules)Tests here: https://regex101.com/r/vIRDKR/8

There is a Version select at the top where you can change to latest... Currently im not into a PR. So next to my regex thing don't consider more help.

Best Michael

sgentile commented 7 years ago

good catch - I spent more than an hour trying to figure out why my html template wasn't inlining and throwing aot errors!