Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to deal with versioning #4

Closed
Avaq opened this issue Feb 6, 2017 · 10 comments
Closed

How to deal with versioning #4

Avaq opened this issue Feb 6, 2017 · 10 comments
Labels

Comments

@Avaq
Copy link
Member

Avaq commented Feb 6, 2017

When using Sanctuary-Type-Identifiers to determine whether something is member of a specific desired type, we get a nice advantage: Even if the value was created in a different context (like in another VM or another package), it will still be identifiable as being our type.

However! When the type was created by an outdated package, we currently have no way to tell that this is actually not the desired type, when using the recommended format for @@type (<vendor>/<type>).

I've therefore been thinking to add a version number to my @@type values (eg <vendor>/<type>@<major>). This would allow my type-checks to detect "older" versions of my type, which may no longer satisfy the interface I expect them to have. And it works both ways: An older version of my lib will not try to consume a newer version of its own type.

Is this something you would stand behind @davidchambers? Or does this somehow go against the purpose of @@type?

@Avaq Avaq added the question label Feb 6, 2017
@davidchambers
Copy link
Member

This seems completely reasonable to me.

If we adopt this proposal, I wonder whether the exported function should return a record rather than a string. Ideally it would be of type { namespace :: Maybe String, name :: String, version :: Maybe Integer }. We may not want to depend on a Maybe type, but we could use Nullable instead.

> type([1, 2, 3])
{namespace: null, name: 'Array', version: null}

> type(Future.of(42))
{namespace: 'fluture', name: 'Future', version: 4}

Users of this function could then decide on the appropriate strictness when type checking.

@Avaq
Copy link
Member Author

Avaq commented Feb 6, 2017

I wonder whether the exported function should return a record rather than a string

That would also make it easier to derive a type identifier.

@davidchambers
Copy link
Member

Are you interested in submitting a pull request, @Avaq?

@Avaq
Copy link
Member Author

Avaq commented Feb 6, 2017

To get this clear; your idea is to:

  • Keep @@type as a String, but have type() parse it according to its specification?
    • Specify more strictly what the string MUST be like?
  • Change the type of the @@type value altogether, and simply return it from type()?

@davidchambers
Copy link
Member

Keeping @@type :: String seems slightly nicer to me if we want the version component to be optional. Compare the following:

MyType['@@type'] = 'my-package/MyType';
MyType['@@type'] = {
  namespace: 'my-package',
  name: 'MyType',
  version: null,
};

Which approach do you favour?

@Avaq
Copy link
Member Author

Avaq commented Feb 6, 2017

I prefer to keep it a String. This will also keep this change compatible with the previous spec, just not the export.

About that... How to deal with versioning? ;)

If we ever change the spec, how would a package indicate that it's using an older version on the spec? The same problem occurs in Fantasy Land. Maybe we should have the property named @@type/1 or use something like {'@@type': 'type1://my-package/Type@1'}. Oh dear.

@davidchambers
Copy link
Member

I suggest we wait until we make a breaking change to the @@type format before versioning the @@type property in some way. Hopefully this will never be necessary.

@Avaq
Copy link
Member Author

Avaq commented Apr 12, 2017

ℹ️ This is on my to-do list for https://github.com/fluture-js/Fluture/projects/1

@safareli
Copy link
Member

safareli commented Apr 21, 2017

I prefer to keep it a String. This will also keep this change compatible with the previous spec, just not the export.

MyType['@@type'] = {
  namespace: 'my-package',
  name: 'MyType',
  version: null,
  toString: () => 'MyType' // compatible ✅
};

@davidchambers
Copy link
Member

Relying on a toString method would not strictly be backwards compatible, @safareli, due to this:

the type representative's @@type property (the type identifier) must be a string primitive

Avaq added a commit that referenced this issue May 8, 2017
Closes #4
Closes #8 (supersedes)
Avaq added a commit that referenced this issue May 8, 2017
Closes #4
Closes #8 (supersedes)
Avaq added a commit that referenced this issue May 8, 2017
Closes #4
Closes #8 (supersedes)
Avaq added a commit that referenced this issue May 8, 2017
Closes #4
Closes #8 (supersedes)
Avaq added a commit that referenced this issue May 11, 2017
This commit adjusts the specification to allow for the
inclusion of a version number in the type identifier.

It also adds a utility function to the library which
takes a string and parses it as a type identifier to
separate the different parts. This function is
exposed as `type.parse`.

Furthermore, because this changed asked for a more
extensive API documentation, the commit adds validation
of the documentation by doctest.

Closes #4
Closes #8 (supersedes)
Avaq added a commit that referenced this issue May 11, 2017
This commit adjusts the specification to allow for the
inclusion of a version number in the type identifier.

It also adds a utility function to the library which
takes a string and parses it as a type identifier to
separate the different parts. This function is
exposed as `type.parse`.

Furthermore, because this chang asked for a more
extensive API documentation, the commit adds validation
of the documentation by doctest.

Closes #4
Closes #8 (supersedes)
Avaq added a commit that referenced this issue May 11, 2017
This commit adjusts the specification to allow for the
inclusion of a version number in the type identifier.

It also adds a utility function to the library which
takes a string and parses it as a type identifier to
separate the different parts. This function is
exposed as `type.parse`.

Furthermore, because this change asked for a more
extensive API documentation, the commit adds validation
of the documentation by doctest.

Closes #4
Closes #8 (supersedes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants