astro check reports that a used function is not used

[jyasskin]

What version of astro are you using?

1.9.2

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

pnpm

What operating system are you using?

Linux

Describe the Bug

Running astro check on:

---
function noSuchRecipe(): Response {
  return new Response("<p>No such recipe</p>", {
    status: 404,
    statusText: "No such recipe",
    headers: {'content-type': 'text/html'},
  });
}

// Grab this recipe.
const { username, slug } = Astro.params;
if (!slug || !username) {
  return noSuchRecipe();
}
---

<p></p>

Returns:

/home/projects/github-urvrsi/src/pages/index.astro:2:10 Hint: 'noSuchRecipe' is declared but its value is never read.

but running the file actually shows the "No such recipe" output.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-urvrsi?file=src/pages/index.astro

Participation

• I am willing to submit a pull request for this issue.

[Princesseuh]

Thank you for reporting this issue! This effectively a bug.

This happens because of the shape of the generated TSX output, since the frontmatter is technically at the top level, the return is considered invalid and thus the function usage too, so it ends up reporting as unused.

Moving this to the language-tools repo since this is ultimately a language-server issue (which also affects the editor, though in different ways)

[Mrowa96]

Hi, any plans to fix it in near future? Right now the only workaround is to suppress error by using @ts-expect-error which is far from perfect ;/

I'm willing to submit a PR but I would need some hints where to start.

[Princesseuh]

This is unfortunately a huge amount of work. We need to change the shape of the TSX output, which implies changing mappings, which tends to imply changing some manual mappings in completions / code actions as well.

If you're interested in tackling this, I can give further information, but it's a fair amount of work, in multiple programming languages and with also a fair amount of background needed 😅

[Mrowa96]

Ok, so for now I will leave it, I will start with something minor 😅

[Princesseuh]

A non-technical alternative would be to create a proposal for this to be changed to a throw, instead of a return. You could create a proposal here for this: https://github.com/withastro/roadmap/discussions

Even it does look weird to throw things like that, it's actually a common pattern when wanting to top-level react to things and would be more valid JavaScript.

[mb21]

Would this be easier to solve if we had script tags instead of frontmatter?

[Princesseuh]

Would this be easier to solve if we had script tags instead of frontmatter?

Nope, same problem. From the editor tooling perspective, it being a script tag or a frontmatter doesn't really change much

[mb21]

I see. Apart from throwing or exposing some sort of main function to be called (which both seem awkward), I can see only one solution: Astro.response (or something like it) should be assignable and of type ResponseInit | Response so you could do:

if (!slug || !username) {
  Astro.response = noSuchRecipe();
  return
}

[mb21]

since the frontmatter is technically at the top level

Or perhaps that's the root problem? Couldn't this be transparently fixed by astro by wrapping the frontmatter in a function (e.g. an Immediately Invoked Function Expression)?

[Princesseuh]

since the frontmatter is technically at the top level

Or perhaps that's the root problem? Couldn't this be transparently fixed by astro by wrapping the frontmatter in a function (e.g. an Immediately Invoked Function Expression)?

Yes and no, for two reasons:

• You can have exports, so you need to hoist them out of the function (which we do in Astro itself, but it's a different idea here since it's types being exported)
• You need to access a local type in the final function signature. You can't export it through a property of the IIFE because TypeScript will consider that you're using a private type outside of its scope.

It's honestly a problem that's way more complicated than it seems

[mb21]

Okay, not sure I understood that 100% as I'm not familiar with the .astro compiler... (I was thinking of a hack to work on the string before it even is parsed into any AST... basically splitting the front-matter string after the imports, then wrapping the second part in a IIFE)

But anyway, if it's not doable, then I vote for Astro.response being assignable... perhaps using a proxy?

[omidantilong]

Just chiming in to say I've also encountered this, and it is very odd. In my case, I was able to suppress the TS error in the IDE (in VSCode at least) by re-assigning the import:

import someResponse from "../someFile"
const fancyResponse = someResponse
...
return fancyResponse()

It's more than a little clunky, but keeps the editor happy. However, astro check will still think fancyResponse is declared but never read.

I've not spent a huge amount of time with Astro, but so far I'm liking it - this is the first real ickiness I've encountered ❤️

[OrlandoBricenoB]

This is happening to me with astro check:

dist/_worker.js/chunks/pages/index_DliSBx1j.mjs:24:25 - warning ts(6133): 'receiver' is declared but its value is never read.

24       get(target, name, receiver) {
                           ~~~~~~~~

With other 13 hints more.

[arty-name]

@OrlandoBricenoB I believe in your case astro check goes through the built code in the dist folder, which is a separate issue

[gersomvg]

If you're open to a simple (hacky) heuristic that might be less work to implement, we could add a compiler step that takes any return x (that is not in a nested function scope) and turns it into const HASH123 = x; HASH123; return HASH123;.

Though that wouldn't solve the error from still showing up in my editor intermittently. Weird that it doesn't do that consistently 🤔

[gersomvg]

A non-technical alternative would be to create a proposal for this to be changed to a throw, instead of a return. You could create a proposal here for this: https://github.com/withastro/roadmap/discussions

I see there is already a proposal for this: withastro/roadmap#830

[Eptagone]

Same problem here. I had to remove astro check from build process because otherwise, it always fails.

[lhansford]

I'm using this workaround until there's a fix. It's a little bit of extra code but allows me to keep astro check in place.

I create a function that I can pass the function that astro check is flagging to. Doing so means that the checker now believes the function to be used.

export function avoidAstroTypeCheckBug(_: unknown) {
  // See https://github.com/withastro/language-tools/issues/476 for more context.
  return;
}

// In a .astro file
import { avoidAstroTypeCheckBug } from './avoidAstroTypeCheckBug';
import { createNotFoundResponse } from './createNotFoundResponse';

if (!someValue) {
  avoidAstroTypeCheckBug(createNotFoundResponse);
  return createNotFoundResponse();
}

[millette]

I had someting very similar to OP:

function noSuchRecipe(): Response {
  return new Response("<p>No such recipe</p>", {
    status: 404,
    statusText: "No such recipe",
    headers: {'content-type': 'text/html'},
  });
}

// Grab this recipe.
const { username, slug } = Astro.params;
if (!slug || !username) {
  return noSuchRecipe();
}

My workaround for now was to rename noSuchRecipe to _noSuchRecipe to silence the warning.

[gersomvg]

I'm using this workaround until there's a fix. It's a little bit of extra code but allows me to keep astro check in place.

I create a function that I can pass the function that astro check is flagging to. Doing so means that the checker now believes the function to be used.

export function avoidAstroTypeCheckBug(_: unknown) {
  // See https://github.com/withastro/language-tools/issues/476 for more context.
  return;
}

// In a .astro file
import { avoidAstroTypeCheckBug } from './avoidAstroTypeCheckBug';
import { createNotFoundResponse } from './createNotFoundResponse';

if (!someValue) {
  avoidAstroTypeCheckBug(createNotFoundResponse);
  return createNotFoundResponse();
}

You could just do:

createNotFoundResponse;
return createNotFoundResponse();

I guess yours is more explanatory

[Princesseuh]

I'm working on a fix here withastro/compiler#1028

[gersomvg]

@Princesseuh Amazing! I guess that means throw syntax will also be explicitly supported? I've been using that since a few weeks with my own middleware and I haven't run into any difficulties. Would be nice to have it natively supported ❤️

[Princesseuh]

@Princesseuh Amazing! I guess that means throw syntax will also be explicitly supported? I've been using that since a few weeks with my own middleware and I haven't run into any difficulties. Would be nice to have it natively supported ❤️

No, I'm only working on making it so top-level returns don't trigger an error in editor tooling + astro check, no changes to Astro itself.

[lhansford]

🎉 thanks @Princesseuh!

[Princesseuh]

Re-opening this as I'm about to revert the fix. It did work for most cases, but some cases broke it horribly and led to confusing errors. Currently trying to work on a fix, but it is quite tricky.

Apologies for the inconvenience!

[millette]

Thanks for the hard work @Princesseuh

Going back to my workaround #476 (comment)

[mlafeldt]

My workaround for now was to rename noSuchRecipe to _noSuchRecipe to silence the warning.

Good idea! Works for me too.

-const { userId, redirectToSignIn } = Astro.locals.auth()
+const { userId, redirectToSignIn: _redirectToSignIn } = Astro.locals.auth()
 if (!userId) {
-  return redirectToSignIn()
+  return _redirectToSignIn()
 }