Fix gjs/gts bugs for ember-template-imports and ember-cli-babel #408
Conversation
|
|
||
| let counter = 0; | ||
|
|
||
There was a problem hiding this comment.
super minor, extra spaces
There was a problem hiding this comment.
Done @SergeAstapov
|
If you wanted to include the workflow from this issue #409 in this PR, I would be able to test these pending changes in my app. |
|
On testing I found that if I did a test without When running test with |
|
@cah-brian-gantzler so you are saying you already tested it on my changes and only problem you found right now is that for you it doesn't work w/o |
|
@cah-brian-gantzler tbh I don't think any of my changes have anything to do with your Also I it's hard for me to debug as I have no way of reproducing that, since on brand new classic/embroider apps it just works normal w/o |
|
@vstefanovic97 Im sorry I thought you were aware, yes, the problem with the Yes I was able to apply your branch to my code and it did produce the expected results for .gjs/.gts |
|
Ok then I guess we are good with my changes, thanks for the confirmation @cah-brian-gantzler! |
| @@ -20,11 +20,12 @@ const gjsGtsTemplateIgnoreVisitor = { | |||
| const { node } = path; | |||
| let { callee } = node; | |||
|
|
|||
| if (callee.name !== 'template') { | |||
| // if there is already a `template` varaible in scope content-tag will use `template1` local name and so on. | |||
There was a problem hiding this comment.
| // if there is already a `template` varaible in scope content-tag will use `template1` local name and so on. | |
| // if there is already a `template` variable in scope, content-tag will use `template1` local name and so on. |
There was a problem hiding this comment.
Type is fixed @SergeAstapov, also I made everything just one commit
| template(); | ||
| counter++; | ||
| console.log(counter); | ||
| } | ||
|
|
||
| export default template(\` | ||
| <template> |
There was a problem hiding this comment.
@vstefanovic97 I may be wrong, but this change make this test not match it's title? like it says if template is not imported from "@ember/template-compiler" but with this change there is no @ember/template-compiler in play... or maybe test title needs to be updated?
There was a problem hiding this comment.
@SergeAstapov the template string is being passed to content-tag to be transpiled now
See here
But what this test is doing is making sure that this doesn't get an ignore comment added
There was a problem hiding this comment.
ahha! Got it now! so we gonna rely on snapshots for this behavior
There was a problem hiding this comment.
yes @SergeAstapov
|
@RobbieTheWagner would you be able to give it a go if all looks good? |
|
Ping @RobbieTheWagner |
RobbieTheWagner
merged commit 9e01d23
into
ember-cli-code-coverage:master
|
@vstefanovic97 does this add template coverage or just fixes gjs/gts files? |
|
It just fixes gjs/gts files @RobbieTheWagner |
|
@RobbieTheWagner do you mind to create a patch release for this fix? |
Just published 2.1.2 |
Changes
fix ember-cli-babel errors
ember-cli-babelparallelizam issues)throwUnlessParallelizable: truein tests to prevent regressionsFix ember-template-imports integration
ember-template-importsused so that files get included in coverage reportgjs-gts-ignore-template-pluginnot being executed in context ofember-template-imports(this was fixed by relying on inputSourceMaps) to get original file extensionembroider + ember-template-importsember-template-imports