Skip to content
New issue

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

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

Already on GitHub? # to your account

Tag and release v4.2.0? #50

Open
hexchain opened this issue Mar 7, 2025 · 11 comments
Open

Tag and release v4.2.0? #50

hexchain opened this issue Mar 7, 2025 · 11 comments

Comments

@hexchain
Copy link

hexchain commented Mar 7, 2025

Would it be possible to tag and release version 4.2.0? I'd really like to use the new evaluateRequest API in a project.

@kornelski
Copy link
Owner

Have you tried the beta release? (Under the next tag, IIRC)

@hexchain
Copy link
Author

hexchain commented Mar 8, 2025

Right, I should try that first.

@hexchain
Copy link
Author

I think the library itself generally works fine, but the type definitions from DT is giving me some headaches.

Usually I can use TypeScript declaration merging to fill in the new API myself, but since this library is using default export, it is not possible at the moment (microsoft/TypeScript#14080). To workaround this I had to re-export CachePolicy.

Would it be possible to also provide (or change to) non-default export in this library?

@kornelski
Copy link
Owner

AFAIK non-default export would be a breaking change, so I'd rather not do that. I could add JSDoc style type annotations?

@hexchain
Copy link
Author

Would it be possible to add a non-default export of the same CachePolicy class/namespace, without changing the default export?

@kornelski
Copy link
Owner

Does this help 8467661?

@hexchain
Copy link
Author

Unfortunately it doesn't seem to help. TypeScript doesn't pick up the type annotations when this package is installed as a dependency.

I'm not sure if TypeScript compiler ignores node_modules by default, or it just doesn't check JS files in there (I have tried setting allowJs: true, adding node_modules/http-cache-semantics/index.js to files in tsconfig.json, but none made a difference). The best bet, I guess, might be to generate a .d.ts before publishing, and let TypeScript know about that through "types" in package.json?

@hexchain
Copy link
Author

Also, I believe that the return type definition for evaluateRequest might be a bit too broad.

The current definition is:

{
  response: { headers: Record<string, string>} | undefined,
  revalidation: { headers: Record<string, string>, synchronous: boolean } | undefined
}

For example, if revalidation does not exist in the return value, there must be a response. So a better one (in native TypeScript) might be:

interface EvaluateRequestHitWithoutRevalidationResult {
  response: {
    headers: CachePolicy.Headers;
  };
  revalidation: undefined;
}

interface EvaluateRequestHitWithRevalidationResult {
  response: {
    headers: CachePolicy.Headers;
  };
  revalidation: EvaluateRequestRevalidation;
}

interface EvaluateRequestMissResult {
  response: undefined;
  revalidation: EvaluateRequestRevalidation;
}

type EvaluateRequestResult =
  | EvaluateRequestHitWithRevalidationResult
  | EvaluateRequestHitWithoutRevalidationResult
  | EvaluateRequestMissResult;

interface CachePolicy {
  evaluateRequest(req: HttpRequest): EvaluateRequestResult;
}

However this is perhaps too cumbersome to express in one line in the JSDoc.

@kornelski
Copy link
Owner

I've added "types" to package.json, this should help finding them.

@hexchain
Copy link
Author

Sorry, it still doesn't seem to help. Please see this example: https://gist.github.com/hexchain/1b266612ac7bc217dae48526e54d225a


% npm run check

> test@1.0.0 check
> tsc --noEmit

index.ts:1:25 - error TS7016: Could not find a declaration file for module 'http-cache-semantics'. '/tmp/test/node_modules/http-cache-semantics/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/http-cache-semantics` if it exists or add a new declaration (.d.ts) file containing `declare module 'http-cache-semantics';`
1 import CachePolicy from 'http-cache-semantics';
                          ~~~~~~~~~~~~~~~~~~~~~~

Found 1 error in index.ts:1

@hexchain
Copy link
Author

I've opened a PR for DT: DefinitelyTyped/DefinitelyTyped#72265

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

No branches or pull requests

2 participants