[BUGFIX] Contextual component variable transformation#230
[BUGFIX] Contextual component variable transformation#230rajasegar wants to merge 4 commits intoember-codemods:masterfrom
Conversation
package.json
Outdated
| "scripts": { | ||
| "coveralls": "cat ./coverage/lcov.info | node node_modules/.bin/coveralls", | ||
| "debug:integration": "node --inspect-brk ./test/run-test.js", | ||
| "debug:test": "node --inspect-brk ./node_modules/.bin/jest --runInBand --testNamePattern", |
There was a problem hiding this comment.
this seems unrelated to the PR?!
There was a problem hiding this comment.
Yep, sorry my bad, it was meant to be in a separate PR, will remove it.
| let tagName = `${node.path.original}`; | ||
| havingBlockParams = node.program.blockParams.length > 0; | ||
| if (havingBlockParams) { | ||
| currentBlockParams = node.program.blockParams; |
There was a problem hiding this comment.
this doesn't work for nested block params
There was a problem hiding this comment.
I think the corresponding template-lint rule has something like this implemented, or you can have a look at the implicit-this codemod
|
@rajasegar I should have a branch pushed to origin for working on this issue. Let’s collaborate. My implementation keeps track of scope (yours might, but I have looked into it) and takes a more conservative approach to the transform when we can decide if a plain value is being yield or a contextual component. |
|
FWIW with the new path param of the visitor this kind of tracking becomes a lot easier 😉 |
|
@Turbo87 thats awesome. I haven’t had a chance to catch up. My current implementation was fairly straight forward, would like your thoughts on it anyway. Be that as it may might point was to make sure @rajasegar knew we could collaborate before he went any further or he can continue fixing it if he likes. |
|
quick question on this: Isn't it this supposed to be a feature? looking at this ember doc https://guides.emberjs.com/release/upgrading/current-edition/templates/ The problem I see is that the codemod doesn't do this consistently |
4 participants
Fixes #220
I set out to solve the blockParams problem , now I have one more failing test case
@tylerturdenpants @Turbo87 Anything I am missing here?