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

Accept partial Order object #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pouwerkerk
Copy link

This PR simply reduces the argument type requirements of getOrder(order: Order) from an object of type Order, to simply an object containing a URL (effectively Pick<Order, 'url'>). Only the Order URL is needed to retrieve an Order and re-create object, and that's how api uses getOrder anyway. Since api isn't exposed to consumers of acme-client, I propose this modest change.

Here's an example use-case:

const acme = require('acme-client');

const accountPrivateKey = '<PEM encoded private key>';

const client = new acme.Client({
    directoryUrl: acme.directory.letsencrypt.staging,
    accountKey: accountPrivateKey,
});

client.getOrder({ url: 'https://acme-provider.example.com/order/123' });

Unfortunately, TypeScript will warn the following:

Type '{ url: string; }' is missing the following properties from type 'Order': status, identifiers, authorizations, finalize

Looking at the type definition of the acme-client order, I see that it actually extends the rfc8555.Order definition:

export interface Order extends rfc8555.Order {
    url: string;
}

And creating a rfc8555.Order just to satisfy the function argument type requirements is onerous:

export interface Order {
    status: 'pending' | 'ready' | 'processing' | 'valid' | 'invalid';
    identifiers: Identifier[];
    authorizations: string[];
    finalize: string;
    expires?: string;
    notBefore?: string;
    notAfter?: string;
    error?: object;
    certificate?: string;
}

Thanks for your consideration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant