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

Automatic detection of JS breaking changes #22983

Open
deyaaeldeen opened this issue Aug 23, 2022 · 2 comments
Open

Automatic detection of JS breaking changes #22983

deyaaeldeen opened this issue Aug 23, 2022 · 2 comments
Assignees
Labels
apiguard API Guard is a tool to detect breaking changes between two APIs

Comments

@deyaaeldeen
Copy link
Member

deyaaeldeen commented Aug 23, 2022

API Guard - JS

Breaking client code degrades our customer experience and will likely hurt Azure adoption. Such breaking changes must be detected before making a release and be handled according to the Azure SDK design guidelines:BreakingChangeGuideline

Breaking changes can be classified into source and behavioral breaking changes:

  • Source breaking changes cause compilation errors next time the client code gets re-compiled.
    • Example: making an optional property in an input model required.
  • Behavioral breaking changes alter the program behavior in an unexpected way.
    • Example: throwing an error in a code path that didn't throw before.

This work tracks building apiguard, a tool to detect source breaking changes between two input extracted APIs.

Design

The tool contains the following components:

  • parsing: rich JSON parser to parse the two input api.json files into a TypeScript code model
  • diffing two code models into a change list model
  • breaking changes rules
  • APIView plugin that understands the flagged breaking changes to highlight them

Parsing

We use the API Extractor to generate *.api.json files that gets uploaded to https://apiview.dev/ and it is powered by @microsoft/api-extractor-model, a library for reading and writing those *.api.json files. This library can be leveraged to do the desired parsing.

Diffing

TBD

Rules list

TBD

APIView plugin

TBD

@deyaaeldeen deyaaeldeen self-assigned this Aug 23, 2022
@deyaaeldeen deyaaeldeen added the apiguard API Guard is a tool to detect breaking changes between two APIs label Aug 23, 2022
@xirzec
Copy link
Member

xirzec commented Aug 29, 2022

Making an attempt at pinning down some heuristics. I'm ready to be heavily edited/corrected for spec correctness.

An interface/type is considered a "return type" if it:

  1. Is returned by a method
  2. Is a publicly available property on a client or client-like

An interface/type is considered an "input type" if it:

  1. Is used as the type of a parameter to a method or constructor

An interface/type may be used as both a return type and an input type.
A type may be anonymous and not have a name (e.g. foo(): A | B returns an anonymous type)

Now some specific rules:

  1. Any rename of a public type or property IS considered breaking.
    • A special case of this is if the rename is only affecting type names and would not be observable to a JS consumer
// Bar could be renamed and not break JS consumers, but it should still be flagged as breaking TS consumers.
interface Bar {
   name: string;
}
function foo() : Bar;
  1. Adding an additional required property on an input type IS considered breaking.
  2. Changing a required property to be optional on a return type IS considered breaking.
  3. Narrowing an input type (e.g. removing a member of a type union) IS considered breaking
  4. Widening a return type (e.g. returning a new shape or extending a type union) IS considered breaking.

I think a lot of the complexity here will be spidering through nested types, e.g.

interface Feature {
   name: string;
   // adding a new required property to `Feature` is breaking even though it is not directly used as an input
}
interface Widget {
  features: Feature[];
}
class FactoryClient {
   produce(widget: Widget): ProductionResponse;
}

azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this issue Mar 15, 2023
Merge release sentinel 2023 03 01 preview (Azure#22983)

* Adds base for updating Microsoft.SecurityInsights from version preview/2023-02-01-preview to version 2023-03-01-preview

* Updates readme

* Updates API version in new specs and examples

* Update description of AAD (Azure#22508)

* Update description of AAD

AAD should be AADIP to prevent mistakes from customer

* Add custom word AADIP 

AADIP = Azure Active Directory Identity Protection

* Remove bing phishing feed msti connector (Azure#22706)

* remove bing phishing feed from msti connector

* revert to release-Sentinel-2023-03-01-preview

* fixes

* fixed validation issues

* renamed example file

* added response status codes for examples

* Automation rules - 2023-03-01-preview - add entity trigger API (Azure#22523)

* adjust version order

* Fix cross-version breaking changes.

Fix cross-version breaking changes.

* Pull in metadata changes from stable version.

* Update Metadata.json

* Update Incidents.json

add readonly flag for providerIncidentId.

* Update readme.md

GuidUsage AutoRest-Linter suppression.

* Update readme.md

suppress GuidUsage

* Update readme.md

Correct suppression.

* Update readme.md

Fix suppression.

---------

Co-authored-by: shschwar <55053268+shschwar@users.noreply.github.com>
Co-authored-by: aviyerMSFT <87452453+aviyerMSFT@users.noreply.github.com>
Co-authored-by: loriatarms <105870291+loriatarms@users.noreply.github.com>
@deyaaeldeen
Copy link
Member Author

@timovv I think you have a simpler design to address the goal documented above. Is it documented somewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apiguard API Guard is a tool to detect breaking changes between two APIs
Projects
None yet
Development

No branches or pull requests

2 participants