Detecting API breaking on Cristal

Hello all,

I’d like to introduce tool support to the detection of API issues in the Cristal packages.

This should help us answer the following questions when changing Cristal code:

  • Is my change breaking existing use of the impacted method?
  • Are all the types necessary to use my method exported as well?
  • Is it safe to use the exported method, or is it still considered unstable?
  • Is my stable code dependent on unstable types?

For those of you familiar with the Java world, we are talking about an equivalent of revapi.

I’d like to propose the introduction of the API extractor library. From my investigations, this is the only tool with active development by a large company (Microsoft). All the other tools I could find were abandoned or were never published to a repository.

Presentation

API extractor allows for the generation of a markdown file from the type signatures of a project, with two modes:

  • local: allows for the modification of the markdown file after local code change
  • check: fails if the markdown file diverges from what’s in the type definitions of the package

Example of the produced markdown:

## API Report File for "@xwiki/cristal-api"

> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).

```ts

import { App } from 'vue';
import { AsyncComponentLoader } from 'vue';
import { AsyncComponentOptions } from 'vue';
import { Component } from 'vue';
import { ComponentPublicInstance } from 'vue';
import { Configurations } from '@xwiki/cristal-configuration-api';
import { Container } from 'inversify';
import { Ref } from 'vue';
import { Router } from 'vue-router';
import { UserDetails } from '@xwiki/cristal-authentication-api';

// @public
export type AttachmentsData = {
    attachments: PageAttachment[];
    count?: number;
};

// @public
export class ComponentInit {
    constructor();
}

// @public (undocumented)
export type ConfigObjectType = {
    name: string;
    baseURL: string;
    baseRestURL: string;
    homePage: string;
    serverRendering: boolean;
    designSystem: string;
    offline: boolean;
    realtimeURL?: string;
    authenticationBaseURL?: string;
    authenticationManager?: string;
    storageRoot?: string;
    editor?: string;
};

// (No @packageDocumentation comment for this package)

```

They would be used in the following ways:

  • For code review: check the changes in API extractor markdown files. This is easier than going through the whole code changes and follow imports.
  • On the CI: run API extractor in `check` mode to detect breackage the are not reflected in the markdown file
  • For developers: this is another view on the public interface of a package. We might also find a way to derive documentation automatically from those files

Definition of the code levels

API extractor propose 4 levels of stability: alpha, beta, public, internal. To match what we do in XWiki, I propose to:

  • never use the alpha level
  • use the beta level in the same way as the unstable annotation (i.e., for newly introduced code)
  • use public level for stable code (i.e., code that exists since a certain period of time and stayed stable)
  • use internal the same way as the internal packages in java (i.e., code that can be imported between packages of the cristal monorepo, but not externally)

Limitations

  • it is not possible to define a default level, so we are going to go through all our code to add @unstable annotations on all the tsdoc (I’m volunteering to do the change, but only once we clearly agree on this proposal as it’s time consuming)
  • we do not have a clear way to detect and fail the build when the @unstable annotation is applied on code with an old version (but I suggest to cover this topic on another proposal once the tool is deployed). This is especially a complex question since we do not have the notion of cycle for Cristal yet. See also the discussion for a 1.0.0 version of Cristal Cristal - XWiki Forum

WDYT?

I’m +1 as my experiments indicated that it was making it easier to understand what is exported. Alnd already allowing for the detection of code exported by accident, or dependent code missing from the exports. Even if the main benefits are to be seen later when some of our code becomes stable.

The experiment sounds interesting but I’m not sure to understand why starting to use @unstable annotation everywhere if you don’t yet have the concept of cycle for Cristal. The whole purpose of @unstable is to be able to remove that annotation after a while: if everything is unstable it defeats the purpose entirely…

I agree. But we need to start somewhere.

To be clear, the goal is to have a rule to replace the unstable @beta with stable @public soon.
But since it’s going to be a long discussion, I prefer it in a separate proposal.

ok, and what about really experimenting while nothing is fixed? Like artificially putting some API to public, knowing they will be broken, just to see how it goes with the CI check, and how to handle properly the changes? Btw is there a way to ignore some errors when doing the check? Like we do with revapi.ignore?

While everything is unstable, we can keep experimenting. But at least we’ll be more easily aware of breaking changes, and agree on them during code review.

I did those tests locally.

The way it works is a bit different (and less powerful than revapi, but there is no better option afaik)

  1. the CI will fail when it detects an inconsistency between the sources and the api extractor markdown file
  2. developers are expected to run api extractor in local mode before pushing a PR
  3. reviewers are expected to inspect the modified api extractor markdown file and validate that the changes are ok

So basically, accepting a change is merging with an updated markdown file.

1 Like

Done. See CRISTAL-645: Add the ability to detect API changes (#1193) · xwiki-contrib/cristal@3e0308b · GitHub