Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

[Storyboards] Change .controller to .instantiate() #65

Merged
merged 10 commits into from
Aug 8, 2017

Conversation

AliSoftware
Copy link
Collaborator

@AliSoftware AliSoftware commented Aug 7, 2017

@djbe as discussed in Slack earlier today. I'll let you run that template on the codebase you tested earlier to see if it's better. Will add CHANGELOG and MG entries later (after GoT 📺⚔️ 😄)

Note that this is a breaking change from 2.0 but given that 2.0 is used by nobody yet (as templates 2.0 requires SwiftGen 5.0 which isn't released yet) we'll probably just tag a 2.0.1. It's not really semver-correct but given 2.0 isn't really officially out and was useless without SwiftGen 5 I feel that it's ok.

TODO:

  • update the code samples in
    • templates docs
    • main repo README
    • main repo playground
  • Add CHANGELOG entry
  • Mention the change from instantiateFoo to Foo.instantiate + the compatibility helper template
  • Mention the param.module in the storyboard templates documentation (could be done in a separate PR as it's unrelated — it just happens that I just realised when re-reading those docs that we missed it, not sure it's worth its own PR though)

Copy link
Member

@djbe djbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works in 2 tested codebases.
Todo: update the code samples in templates docs, main repo readme, and playground.

The GoT episode was on fire! :trollface:

@AliSoftware
Copy link
Collaborator Author

AliSoftware commented Aug 7, 2017

@djbe thought: provide an additional template with @deprecated attributes to help the migration, e.g.

@available(*, deprecated, rename: "initialScene.instantiate()")
static func instantiateInitialScene() -> {{sceneClass}} { return initialScene.instantiate() }
📑 Compatibility template example (proof of concept) — open to see code
// Generated using SwiftGen, by O.Halligon — https://github.com/SwiftGen/SwiftGen
{% if platform and storyboards %}
{% set isAppKit %}{% if platform == "macOS" %}true{% endif %}{% endset %}
{% set prefix %}{% if isAppKit %}NS{% else %}UI{% endif %}{% endset %}
{% set controller %}{% if isAppKit %}Controller{% else %}ViewController{% endif %}{% endset %}

import Foundation
import {% if isAppKit %}Cocoa{% else %}UIKit{% endif %}
{% for module in modules where module != env.PRODUCT_MODULE_NAME and module != param.module %}
import {{module}}
{% endfor %}

{% set sceneEnumName %}{{param.sceneEnumName|default:"StoryboardScene"}}{% endset %}
{% macro className scene %}{% filter removeNewlines %}
  {% if scene.customClass %}
    {% if scene.customModule %}{{scene.customModule}}.{% endif %}
    {{scene.customClass}}
  {% else %}
    {{prefix}}{{scene.baseType}}
  {% endif %}
{% endfilter %}{% endmacro %}

enum {{sceneEnumName}} {
  {% for storyboard in storyboards %}
  {% set storyboardName %}{{storyboard.name|swiftIdentifier|escapeReservedKeywords}}{% endset %}
  enum {{storyboardName}} {
    {% if storyboard.initialScene %}
    {% set sceneClass %}{% call className storyboard.initialScene %}{% endset %}
    @available(*, deprecated, rename: "initialScene.instantiate()")
    static func instantiateInitialScene() -> {{sceneClass}} { return initialScene.instantiate() }
    {% endif %}

    {% for scene in storyboard.scenes %}
    {% set sceneID %}{{scene.identifier|swiftIdentifier|snakeToCamelCase|lowerFirstWord|escapeReservedKeywords}}{% endset %}
    {% set sceneClass %}{% call className scene %}{% endset %}
    @available(*, deprecated, rename: "{{sceneID}}.instantiate()")
    static func instantiate{{sceneID|titlecase}}() -> {{sceneClass}} {
      return {{sceneID}}.instantiate()
    }
    {% endfor %}
  }
  {% endfor %}
}

{% elif storyboards %}
// Mixed AppKit and UIKit storyboard files found, please invoke swiftgen with these separately
{% else %}
// No storyboard found
{% endif %}

Not tested, but I guess you see the idea. Worth it? (or maybe not as a bundle template, but just paste it somewhere in the wiki or MG and let people copy it manually if they need, to make it less official?)

@AliSoftware
Copy link
Collaborator Author

💡 This made me realise that we didn't document the param.module customisation in the storyboard templates documentation

@djbe
Copy link
Member

djbe commented Aug 7, 2017

Hmmm, I wouldn't add as an alternative template, that's more code we have to maintain. Maybe put it in a wiki page, and point the MG to it. Mention on that wiki page that it won't be maintained.

It is a small change after all.

@djbe
Copy link
Member

djbe commented Aug 7, 2017

It is documented in the templates docs.

@djbe
Copy link
Member

djbe commented Aug 7, 2017

Oh, you mean the template inferring the modules, gotcha.

@AliSoftware
Copy link
Collaborator Author

I mean that https://github.com/SwiftGen/templates/blob/master/Documentation/storyboards/swift3.md mentions sceneEnumName and segueEnumName in their Customization section, but not module

@AliSoftware
Copy link
Collaborator Author

AliSoftware commented Aug 7, 2017

Hmmm, I wouldn't add as an alternative template, that's more code we have to maintain. Maybe put it in a wiki page, and point the MG to it. Mention on that wiki page that it won't be maintained.

Yeah, hence the idea of just pasting it in a wiki page, not bundling it in SwiftGen. And mentioning it as something like:

Tip: if you need help migrating, you could imagine creating a template like the following, which will generate deprecation warnings in your existing code to help you use the "Fix All in Scope" feature of Xcode and let Xcode do the renaming for you to migrate your existing code faster.

Note: this template below is just a suggestion to ease your migration. It won't be integrated into SwiftGen nor maintained, as it's just a tip to help you when you are gonna do your migration but isn't intended to be used as a final template


It is a small change after all.

Sure but it could be very tedious to have to rename every single call to a StoryboardScene in an existing huge codebase… one by one… by hand. While @available(*, deprecated, rename: "newName") allows Xcode's fix-it to do the renaming for you.

[UPDATE]: Here it is, added in the wiki

@djbe
Copy link
Member

djbe commented Aug 8, 2017

That should be the changes for this PR/repo. @AliSoftware you might want to proofread the changelog entry, and then it should be GTM.

@AliSoftware AliSoftware merged commit 694fdf1 into master Aug 8, 2017
@AliSoftware AliSoftware deleted the feature/breaking/sb-instantiate branch August 8, 2017 22:25
@djbe djbe added this to the 2.0.0 milestone Aug 20, 2017
@djbe djbe removed the status: WIP label Aug 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants