Rename JSON to Value #473

gfontenot · 2017-09-30T16:44:04Z

We're going to be shifting the marketing focus for Argo away from being a
JSON parser and towards being a general purpose parser for transforming
loosely typed data models into strongly typed ones. As a part of this
change, we can generalize our types a bit, and use the name Value for our
input values, instead of JSON. This change is purely cosmetic, but will
result in a more general API, letting us break a bit from our ties to JSON.

Note that we're providing a typealias from JSON to Value here as well.
This should make this a non-breaking change (although users will get a
deprecation warning). I'd love it if other people could test this with their
codebase.

houndci-bot · 2017-09-30T17:02:06Z

Sources/Argo/Functions/decode.swift

@@ -50,7 +50,7 @@ public func decode<T: Decodable>(_ object: Any) -> Decoded<T> where T == T.Decod
  - returns: A `Decoded<[T]>` value where `T` is `Decodable`
 */
 public func decode<T: Decodable>(_ object: Any) -> Decoded<[T]> where T == T.DecodedType {
-  return Array<T>.decode(JSON(object))
+  return Array<T>.decode(Value(object))


Syntactic Sugar Violation: Shorthand syntactic sugar should be used, i.e. [Int] instead of Array. (syntactic_sugar)

mdiep · 2017-10-03T00:09:35Z

This should make this a non-breaking change (although users will get a
deprecation warning).

There's a caveat here: this isn't a source breaking change, but it is a binary breaking change.

jshier · 2017-10-03T00:10:47Z

It would be versioned, so how is that relevant?

mdiep · 2017-10-03T06:31:36Z

We ran into an issue with this with ReactiveCocoa or ReactiveSwift. I can't find the specific GitHub issue, but @andersio or @ikesyo might remember how to find it.

IIRC, it becomes an issue if frameworks are compiled against different versions of Argo.

Imagine you have A.framework and B.framework. Both A and B have declared that they're compatible with Argo 4.1..<5.0. When you compile A, it uses Argo 4.1. When you compile B, it uses a copy of Argo, say 4.2, that includes this change. The compiled versions of A and B cannot be used together.

This will lead to build failures when using Carthage in many cases. (Depending on how A and B are set up.) I'm unsure whether this is a problem in practice with CocoaPods or SwiftPM. Semantic version unfortunately doesn't really address the difference between source compatibility and binary compatibility—that's part of the reason I don't think we should use semver.

gfontenot · 2017-10-03T13:34:47Z

This would only really be an issue if we didn't release this as 5.0, right? I was assuming that the rename changes would come with a 5.0 label because I do see binary breakage as being a breaking change.

jshier · 2017-10-03T15:27:47Z

@mdiep I'm not really sympathetic to that scenario. If you're not properly managing your versions, or you're distributing third party libraries precompiled, it's up to you, not the library, to ensure the versions work together. It's the same reason we don't distribute static Alamofire libraries: we don't want to be bound by whatever binary issues may ensue. To me it's only valid to consider the binary compatibility of libraries that are actually distributed as binaries. Especially until Swift has an actual ABI. At that point managing binary compatibility becomes much much easier.

mdiep · 2017-10-03T16:47:36Z

This would only really be an issue if we didn't release this as 5.0, right? I was assuming that the rename changes would come with a 5.0 label because I do see binary breakage as being a breaking change.

Correct. I only brought it up since you mentioned this was a non-breaking change and this previously caught me be surprise. ☺️

If you're not properly managing your versions, or you're distributing third party libraries precompiled, it's up to you, not the library, to ensure the versions work together.

This isn't about distributing precompiled libraries. It's noting that "breaking change" according to SemVer doesn't distinguish between source-breaking and binary-breaking.

A common way this might arise: A framework includes its dependencies as submodules and builds against the submodule versions. These aren't precompiled. They're just a different version than the one you might ultimately link against.

jshier · 2017-10-03T17:19:46Z

Submodule integration, unless managed by a tool, falls under the "not properly managing your versions" umbrella. To my mind, libraries aren't responsible for version conflicts in a non-version managed environment. Frankly I think SemVer doesn't mention binary compatibility because it assumes ABI stability on the part of the language itself. Swift is rather unusual in that case and the concern will go away after Swift 5.

mdiep · 2017-10-03T23:09:53Z

Submodule integration, unless managed by a tool, falls under the "not properly managing your versions" umbrella.

These submodules can be managed by Carthage.

Frankly I think SemVer doesn't mention binary compatibility because it assumes ABI stability on the
part of the language itself

I'm not sure those are the same thing. AIUI ABI stability guarantees compatibility between different versions of the compiler, which is not necessarily the same thing as guaranteeing that a source-compatible change is always a binary-compatible change. (As evidenced by the fact that this problem occurs when using the same version of swift for all dependencies.)

Frankly I think SemVer doesn't mention binary compatibility because it assumes ABI stability on the part of the language itself.

I think the more likely explanation is that Tom Preston-Werner (SemVer's author) is primarily experienced in Ruby and JavaScript. ☺️

gfontenot · 2018-04-25T20:22:01Z

7 months seems like it's long enough to wait for feedback on a change like this, right? Any last-minute objections?

jshier · 2018-04-25T21:30:55Z

Given that the Swift team seems to see no importance in being able to decode anything but Data, has thought been given to how this change, or Argo in general, could be used as a Codable wrapper for Any and such? Or is that planned already?

gfontenot · 2018-04-25T21:35:31Z

I don't think I've put much thought into Codable for Any at all, but I do think it's worth re-framing Argo as a more generic library for converting from weakly typed data structures into strongly typed ones. That sure sounds like what you're describing though.

jshier · 2018-04-25T21:46:22Z

Yes. There's been some discussion on the Swift forum regarding how to handle unstructured values in regards to decoding JSON (and like other decoding situations). Some users had created a JSON enum almost exactly like Argo, and I think someone had made Argo's JSON Decodable as well. It would be nice if the new Argo Value type could fill that niche OOTB.

sharplet

Just throwing this out there: what do you think about the name AnyValue? There's lots of precedent in Swift for weakly-typed values using the Any prefix (AnyObject, AnyKeyPath, AnyHashable, AnySequence). I kinda like the idea that AnyValue encodes a value with a dynamic structure.

gfontenot · 2018-04-26T15:22:08Z

Oh, I like AnyValue. My brain thinks of Any* types to be type erasers, though, where as this is more of a type... pencil? What's the dual of a type eraser? Identifier?

gfontenot · 2018-04-26T15:24:49Z

@jshier I dig the direction you're going, fwiw. I don't want to derail this conversation though. Want to open a new issue with a proposal (if you have one, or a basic discussion starter if you don't)? It seems like this might dovetail with #481, too.

sharplet · 2018-04-26T16:10:42Z

My brain thinks of Any* types to be type erasers, though

Yeah, I'm not sure AnyValue is quite right. Although I do think it would read nicely at the point of use:

static func decode(_ v: AnyValue) -> Decoded<Foo> {
  // ...
}

Things I like about it:

Communicates that the thing you have has a weaker type than the thing you want
Succinct
Idiomatic

Possible concerns:

It's not really a type eraser
Possibly implies that you can dynamic cast, which is true for things typed Any, AnyHashable and AnyKeyPath (but is not true for AnySequence or AnyCollection)

jshier · 2018-04-26T16:12:33Z

It's also not "Any" value but a limited subset of values. Even if the supported types expand beyond the current design, "Any" will always be a bit misleading.

sharplet · 2018-04-26T16:14:01Z

It's also not "Any" value but a limited subset of values.

IMO the idea is less that it could be any value, and more that it's a valid in-memory encoding of any value.

houndci-bot · 2018-04-29T15:27:25Z

Sources/Argo/Types/Value.swift

+  }
+}
+
+


Vertical Whitespace Violation: Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)

We're going to be shifting the marketing focus for Argo away from being a JSON parser and towards being a general purpose parser for transforming loosely typed data models into strongly typed ones. As a part of this change, we can generalize our types a bit, and use the name `Value` for our input values, instead of `JSON`. This change is purely cosmetic, but will result in a more general API, letting us break a bit from our ties to JSON.

This should make the JSON -> Value rename a non-breaking one. Existing code using the JSON type will continue to work, but will also prompt users to switch to the Value type.

mdiep · 2018-04-29T20:01:41Z

Would it make sense to be generic over some Value type instead?

JSON really is JSON-specific. You might want to work with a different set of primitive types. e.g. if you wanted to work with a plist, you might add date and data and rename object to dictionary. JSON could be left as-is to support JSON out-of-the-box.

That would also require adding format-specific Decodable protocols, which is a little bit of a pain, but not too bad.

houndci-bot reviewed Sep 30, 2017

View reviewed changes

gfontenot mentioned this pull request Sep 30, 2017

RFC: Renaming the Decodable protocol #474

Open

gfontenot mentioned this pull request Jan 30, 2018

RFC: Renaming (or removing) the <| family of operators #479

Closed

gfontenot force-pushed the gf-json-rename branch from 05e0037 to 44fd5f1 Compare April 25, 2018 20:20

sharplet reviewed Apr 26, 2018

View reviewed changes

gfontenot force-pushed the gf-json-rename branch 2 times, most recently from e2e2aee to c7b6c93 Compare April 29, 2018 15:25

houndci-bot reviewed Apr 29, 2018

View reviewed changes

gfontenot added 2 commits April 29, 2018 10:28

Add deprecation warning for JSON type

1235b45

This should make the JSON -> Value rename a non-breaking one. Existing code using the JSON type will continue to work, but will also prompt users to switch to the Value type.

gfontenot force-pushed the gf-json-rename branch from c7b6c93 to 1235b45 Compare April 29, 2018 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename JSON to Value #473

Rename JSON to Value #473

gfontenot commented Sep 30, 2017

houndci-bot Sep 30, 2017

mdiep commented Oct 3, 2017

jshier commented Oct 3, 2017

mdiep commented Oct 3, 2017

gfontenot commented Oct 3, 2017

jshier commented Oct 3, 2017

mdiep commented Oct 3, 2017 •

edited

Loading

jshier commented Oct 3, 2017

mdiep commented Oct 3, 2017

gfontenot commented Apr 25, 2018

jshier commented Apr 25, 2018

gfontenot commented Apr 25, 2018

jshier commented Apr 25, 2018

sharplet left a comment

gfontenot commented Apr 26, 2018

gfontenot commented Apr 26, 2018

sharplet commented Apr 26, 2018

jshier commented Apr 26, 2018

sharplet commented Apr 26, 2018

houndci-bot Apr 29, 2018

mdiep commented Apr 29, 2018

Rename JSON to Value #473

Are you sure you want to change the base?

Rename JSON to Value #473

Conversation

gfontenot commented Sep 30, 2017

houndci-bot Sep 30, 2017

Choose a reason for hiding this comment

mdiep commented Oct 3, 2017

jshier commented Oct 3, 2017

mdiep commented Oct 3, 2017

gfontenot commented Oct 3, 2017

jshier commented Oct 3, 2017

mdiep commented Oct 3, 2017 • edited Loading

jshier commented Oct 3, 2017

mdiep commented Oct 3, 2017

gfontenot commented Apr 25, 2018

jshier commented Apr 25, 2018

gfontenot commented Apr 25, 2018

jshier commented Apr 25, 2018

sharplet left a comment

Choose a reason for hiding this comment

gfontenot commented Apr 26, 2018

gfontenot commented Apr 26, 2018

sharplet commented Apr 26, 2018

jshier commented Apr 26, 2018

sharplet commented Apr 26, 2018

houndci-bot Apr 29, 2018

Choose a reason for hiding this comment

mdiep commented Apr 29, 2018

mdiep commented Oct 3, 2017 •

edited

Loading