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

generate index.d.ts in build #338

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

katehedgpeth
Copy link

I was using miragejs which builds off of pretender, and noticed that typescript was complaining that pretender.handledRequests does not exist. I discovered that pretender uses a separate index.d.ts file that is not referenced by the actual class. I've opened issue #337 to report this, but I'm also submitting a PR with my suggested fix.

It is possible to simply add class Pretender extends Server, but that would not throw an error if new properties are added to Pretender but not to Server. In my opinion, the best thing to do is to export the types directly from the source files in the build.

This PR updates the rollup config to include all types in the build, using rollup-plugin-typescript2. Doing so also caught a few typescript errors, which I fixed.

I have created a PR on my fork which proves that this change does not affect the generated .js files: katehedgpeth#1

Copy link
Member

@xg-wang xg-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nice. Thank you for putting this up!

Can you also export the remaining types in https://github.com/pretenderjs/pretender/blob/65fbf0a608f2e4b4fd79657a77a41f52077074a9/index.d.ts? This will ensure type compatibility.

Another request is can you annotate the methods that are in the index.d.ts? For example, handledRequest params are currently generated to be typed any, but explicitly typed in index.d.ts. There are couple more there. The concern is there needs another major bump to avoid breaking types if we loosen types in this PR.

Since this repo ignores the generated files, this is new type file I built from this branch:

Click to toggle content
import FakeXMLHttpRequest from 'fake-xml-http-request';
import { Params, QueryParams } from 'route-recognizer';
import { ResponseHandler, ResponseHandlerInstance } from '../index.d';
import Hosts from './hosts';
import parseURL from './parse-url';
import Registry from './registry';
interface ExtraRequestData {
    url: string;
    method: string;
    params: Params;
    queryParams: QueryParams;
}
declare type FakeRequest = FakeXMLHttpRequest & ExtraRequestData;
declare class NoopArray {
    length: number;
    push(..._items: any[]): number;
}
export default class Pretender {
    static parseURL: typeof parseURL;
    static Hosts: typeof Hosts;
    static Registry: typeof Registry;
    hosts: Hosts;
    handlers: ResponseHandler[];
    handledRequests: any[] | NoopArray;
    passthroughRequests: any[] | NoopArray;
    unhandledRequests: any[] | NoopArray;
    requestReferences: any[];
    forcePassthrough: boolean;
    disableUnhandled: boolean;
    ctx: {
        pretender?: Pretender;
    };
    running: boolean;
    private _nativeXMLHttpRequest;
    private _fetchProps;
    constructor();
    get: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    post: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    put: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    delete: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    patch: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    head: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    options: (this: Pretender, path: string, handler: ResponseHandler, async: boolean) => ResponseHandlerInstance;
    map(maps: (pretender: Pretender) => void): void;
    register(verb: string, url: string, handler: ResponseHandler, async: boolean): ResponseHandlerInstance;
    passthrough: {};
    checkPassthrough(request: FakeRequest): boolean;
    handleRequest(request: FakeRequest): void;
    handleResponse(request: FakeRequest, strategy: any, callback: Function): void;
    resolve(request: FakeRequest): void;
    requiresManualResolution(verb: string, path: string): boolean;
    prepareBody(body: any, _headers: any): any;
    prepareHeaders(headers: any): any;
    handledRequest(_verb: any, _path: any, _request: any): void;
    passthroughRequest(_verb: any, _path: any, _request: any): void;
    unhandledRequest(verb: any, path: any, _request: any): void;
    erroredRequest(verb: any, path: any, _request: any, error: any): void;
    shutdown(): void;
    private _handlerFor;
}
export {};

@katehedgpeth
Copy link
Author

katehedgpeth commented Oct 26, 2021

@xg-wang Great suggestion. I've added types to the class methods, and exported the old Server type from pretender.ts. It looks like the build is failing now though, and I'm not sure how to fix it. Do I need to bump the version number?

@@ -2,7 +2,7 @@ import FakeXMLHttpRequest from 'fake-xml-http-request';
import { createPassthrough } from './create-passthrough';

export function interceptor(ctx) {
function FakeRequest() {
function FakeRequest(this: any) {
Copy link
Author

@katehedgpeth katehedgpeth Oct 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really hate using any, but I wasn't totally sure what this should be. Is it Pretender?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should really be rewritten into class :) Seems the generated type is fine, maybe just leave it there for now

@@ -42,6 +42,7 @@
"release-it-lerna-changelog": "^2.3.0",
"rollup": "^2.10.2",
"rollup-plugin-typescript": "^1.0.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to remove this then rerun yarn.

error Your lockfile needs to be updated

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.

2 participants