[ASTextNode2] Provide compiler flag to enable ASTextNode2 for all usages. #410
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ges. The runtime switch is helpful, but is too slow to be shipped in a large production application where startup time is carefully optimized. Although this doesn't pass text-related snapshot tests when enabled, it does allow apps to rely on built-in components like ASButtonNode using the same text stack as when they are manually creating ASTextNode2 instances. A simpler approach would be to use a #define ASTextNode ASTextNode2, but this would create unusual keyword highlighting in code referencing ASTextNode. However, because it would be less invasive and this is not on by default, we could do that instead if preferred.
appleguy
commented
Jul 2, 2017
| @@ -11,6 +11,7 @@ | |||
| // | |||
|
|
|||
| #import <AsyncDisplayKit/ASTextNode2.h> | |||
| #import <AsyncDisplayKit/ASTextNode.h> // Definition of ASTextNodeDelegate | |||
There was a problem hiding this comment.
In the future everything will be merged. However, in a separate diff, we could clean this up to add an ASTextNodeDefinitions.h or similar to declare both the highlight enum and the delegate protocol.
I'm aiming to keep this particular diff quick, but it does raise the question of what our migration path should be to ASTextNode2.
|
🚫 CI failed with log |
|
🚫 CI failed with log |
Adlai-Holler
reviewed
Jul 3, 2017
There was a problem hiding this comment.
To make the relationship between this compile-time flag and the runtime options more clear, lets:
- Add linking comments between the two sites (ASAvailability.h and ASTextNode+Beta.h).
- Rename the macro to something like
ASTEXTNODE_EXPERIMENT_GLOBAL_ENABLEto reinforce the connection.
Beyond that I'm cool with landing this puppy.
bernieperez
pushed a commit
to AtomTickets/Texture
that referenced
this pull request
Apr 25, 2018
…ges. (TextureGroup#410) * [ASTextNode2] Provide compiler flag to enable ASTextNode2 for all usages. The runtime switch is helpful, but is too slow to be shipped in a large production application where startup time is carefully optimized. Although this doesn't pass text-related snapshot tests when enabled, it does allow apps to rely on built-in components like ASButtonNode using the same text stack as when they are manually creating ASTextNode2 instances. A simpler approach would be to use a #define ASTextNode ASTextNode2, but this would create unusual keyword highlighting in code referencing ASTextNode. However, because it would be less invasive and this is not on by default, we could do that instead if preferred. * Update AsyncDisplayKit.h * [ASTextNode2] Improve naming and documentation of ASTEXTNODE_EXPERIMENT_GLOBAL_ENABLE * [ASTextNode2] CHANGELOG.md for TextureGroup#410.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The runtime switch is helpful, but is too slow to be shipped in a large
production application where startup time is carefully optimized.
Although this doesn't pass text-related snapshot tests when enabled, it
does allow apps to rely on built-in components like ASButtonNode using
the same text stack as when they are manually creating ASTextNode2
instances.
A simpler approach would be to use a #define ASTextNode ASTextNode2,
but this would create unusual keyword highlighting in code referencing
ASTextNode. However, because it would be less invasive and this is
not on by default, we could do that instead if preferred.