UIP-1638: Make transformer output strong mode compliant #81
Conversation
…on-abstract components
…ry to the Component class UIP-1638
RavenNumber of Findings: 0 |
Codecov Report
@@ Coverage Diff @@
## master #81 +/- ##
==========================================
+ Coverage 97.57% 97.58% +0.02%
==========================================
Files 29 29
Lines 1394 1402 +8
==========================================
+ Hits 1360 1368 +8
Misses 34 34 |
| @@ -273,6 +277,22 @@ class ImplGenerator { | |||
| ' extends Object with $componentClassImplMixinName' | |||
| ); | |||
| } | |||
|
|
|||
| if (new RegExp(r'typed(Props|State)Factory\(.+\)(\s+)?({|=>)').hasMatch(sourceFile.getText(0))) { | |||
There was a problem hiding this comment.
I think it'd be much more robust to use the AST to detect this.
There was a problem hiding this comment.
Could do something like this:
typedMap.node.members.any((member) => member is MethodDeclaration && !member.isStatic && (
member.name.name == 'typedPropsFactory' ||
member.name.name == 'typedStateFactory'
));There was a problem hiding this comment.
Yeah I like that a lot better! Thanks
| var args = ['--strong'] | ||
| ..addAll(files); | ||
|
|
||
| return await Process.run('dartanalyzer', args); |
There was a problem hiding this comment.
You can just give dartanalyzer a path:
return await Process.run('dartanalyzer', ['--strong', projectPath]);There was a problem hiding this comment.
It keeps timing out when I do that
There was a problem hiding this comment.
Hm, interesting.
I'd try using the project path as the working directory for the command. If that doesn't work, that's fine, but we should have a comment here as to why we're listing out all of the files.
| test('generates strong mode compliant code', () async { | ||
| var testProject = copyProject(strongModeProject); | ||
|
|
||
| // Run `pub get` and the bootstrap up-front so that back-to-back runs on the same directory complete faster. |
There was a problem hiding this comment.
I think this comment was relevant to this code being run in setUpAll, and is no longer applicable here.
| class GenericStatelessProps extends UiProps {} | ||
|
|
||
| @Component() | ||
| class GenericStatelessComponent<T extends GenericStatelessProps> extends UiComponent<T> { |
There was a problem hiding this comment.
We should add a test fixture for a component with generic state as well, to fully test the generation of typedStateFactory in that scenario.
UIP-1638
|
@greglittlefield-wf Feedback has been addressed/replied to |
+10 |
aaronlademann-wf
changed the title
|
QA +1
Merging. |
Ultimate problem:
Code outputted by generated code was not strong-mode compliant under certain circumstances (concrete classes with generic parameter for props/state type).
How it was fixed:
typedPropsFactoryandtypedStateFactoryto get around that.typedPropsFactoryortypedStateFactoryTesting suggestions:
Potential areas of regression: