Workaround to fix classname issues with storyboards and customModule #36
Conversation
templates/storyboards-swift3.stencil
Outdated
| @@ -1,6 +1,7 @@ | |||
| // Generated using SwiftGen, by O.Halligon — https://github.com/SwiftGen/SwiftGen | |||
|
|
|||
| {% macro className scene %}{% if scene.customClass %}{% if scene.customModule %}{{scene.customModule}}.{% endif %}{{scene.customClass}}{% else %}UI{{scene.baseType}}{% endif %}{% endmacro %} | |||
| {% macro className scene %}{% if scene.customClass %}{% if scene.customModule %}{% if not param.ignoreTargetModule or scene.customModule != env.PRODUCT_MODULE_NAME and scene != param.module %}{{scene.customModule}}.{% endif %}{% endif %}{{scene.customClass}}{% else %}UI{{scene.baseType}}{% endif %}{% endmacro %} | |||
|
|
|||
There was a problem hiding this comment.
Also, no need to add this newline.
|
Indeed, I forgot the other templates (sorry). Just updated the PR :) |
There was a problem hiding this comment.
Right, so this branch was quite out of date. With the merge, tests are running correctly now.
I noticed an error in the if test when matching param.module, it isn't testing the correct property.
Once #2 lands, this PR needs to add the needed documentation for the ignoreTargetModule parameter.
I'm wondering if we should add tests for this. @AliSoftware thoughts?
| @@ -1,6 +1,6 @@ | |||
| // Generated using SwiftGen, by O.Halligon — https://github.com/SwiftGen/SwiftGen | |||
|
|
|||
| {% macro className scene %}{% if scene.customClass %}{% if scene.customModule %}{{scene.customModule}}.{% endif %}{{scene.customClass}}{% else %}UI{{scene.baseType}}{% endif %}{% endmacro %} | |||
| {% macro className scene %}{% if scene.customClass %}{% if scene.customModule %}{% if not param.ignoreTargetModule or scene.customModule != env.PRODUCT_MODULE_NAME and scene != param.module %}{{scene.customModule}}.{% endif %}{% endif %}{{scene.customClass}}{% else %}UI{{scene.baseType}}{% endif %}{% endmacro %} | |||
There was a problem hiding this comment.
scene != param.module
should be
scene.customModule != param.module
|
Yeah, we totally should test this, specially given how it seems to be some strange behaviour of Xcode with storyboards which might change again in future Xcode version! |
|
@juli1quere Bump |
|
In fact, I do not have enough knowledge of Swiftgen to write tests that check this issue :/ |
|
I'll write some tests if I can find some time this week, no promises though, might be next week. |
|
Is this still relevant with the new SwiftGen 5.0 templates? If so, could we finish this PR to include it in the next release? |
|
The current templates will always prepend the module to scene classes, even for the target module. In some scenarios, the user might want to disable this, hence this PR. |
|
Hmmm, that's annoying... It can't compile our ignore target module files, because it can't find the custom class. The custom class is in an external module, and the import is ignored... @AliSoftware thoughts? Add something to the filename that says "don't compile this"? |
|
Can't we create a stub-env doing the import or something? |
|
The thing is, the best way to do this is if we could define the name of the If we can define a module when compiling both the output code + the definitions, I think we'll be able to solve this issue by adding the missing class to definitions.swift so that it's both in the "current" module and an imported one. |
|
Allright, @AliSoftware no idea what you'll think of this solution, but it works 😄 |
|
I think at that point basing the decision on the file name is acceptable. But in the long run I hope we won't have more exception and cases to consider. Otherwise we might need to make our build tests evolve e.g. use a YAML or JSON file to describe the matrix of additional options needed for each template or whatnot. But for now let's go with name matching. |
|
@djbe Is this ready to go? I see that you removed the WIP label but didn't notify the maintainers. |
|
Eh, I left it open in case there was more feedback incoming. I suppose this can get merged 👍 |
|
@AliSoftware Sorry to bother you on this PR, but I was wondering if you had an idea of when this fix will get released? |
|
Should be included within the next version of SwiftGen next weekend |
Hey,
Here is the workaround to fix classname issues with storyboards and customModule (as described in #25) :)
Fixes #25