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

Middleware imports are not being resolved properly in a TS project #7017

Closed
1 task done
raulfdm opened this issue May 6, 2023 · 5 comments · Fixed by #7032
Closed
1 task done

Middleware imports are not being resolved properly in a TS project #7017

raulfdm opened this issue May 6, 2023 · 5 comments · Fixed by #7032

Comments

@raulfdm
Copy link
Contributor

raulfdm commented May 6, 2023

What version of astro are you using?

2.4.1

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

None

What package manager are you using?

pnpm

What operating system are you using?

Mac

What browser are you using?

not relevante

Describe the Bug

Hey peps. 👋

First, thanks for finally adding middleware to Astro. This was a feature I had been waiting for so long, mainly because Vercel's middleware is very restrictive in what we can import and do.

Problem

Anyhow, I have an Astro project in TS, and while trying to follow the documentation example (the sequence one, for example), I noted one thing: the types are not being resolved:

Screenshot 2023-05-06 at 10 00 22

That's a bit odd. I went down to Astro's module to see if the exports are declared in the package.json, and actually they are:

Screenshot 2023-05-06 at 10 01 55

If it's declared... I wasn't sure why the heck it wasn't working.

Then, I remembered I did some investigation a while ago about how the conditional types export (exports[number].types) are not always resolved in the IDE, kind of forcing us to rely on a single .d.ts export.

You can check my strategy here

But visiting this, I found a possible solution based on this issue (microsoft/TypeScript#50794) opened on the TypeScript repository.

In summary, "moduleResolution": "node" can't handle these conditional exports and that's the option we have set in Astro's base.json:

Screenshot 2023-05-06 at 10 11 16

@andrewbranch states:

package.json exports are only supported in --moduleResolution node16 and --moduleResolution nodenext. This is expected behavior, but we are working on expanding it. A few points of common confusion:

  • If you’re using Node 12 or 14, you should use --moduleResolution node16. >The features added to 16 have fully or mostly been backported to 12 and 14.
  • --moduleResolution node is for Node 11 and older. Its name is currently bad. We plan to rename it accordingly.
  • If you are using a bundler or a non-Node runtime, we do not currently have a mode for you. node16 and nodenext are only for Node.*

Then, I have overridden the moduleResolution to node16 to see if that would fix the problem, and it did:

Screenshot 2023-05-06 at 10 16 23

Solution

If it's said that Astro's project has to use Node 14+, does it make sense to change the base.json to moduleResolution: node16?

Steps to reproduce

  1. access https://stackblitz.com/github/devraul/astro-middleware-types?file=src/middleware.ts
  2. you'll see that the astro/middleware import isn't working (for TS only);
  3. open tsconfig.json and uncomment the compilerOptions that sets moduleResolution to node16 and save the file;
  4. open the middleware again (src/middleware.ts);
  5. the problem was gone

Link to Minimal Reproducible Example

https://stackblitz.com/github/devraul/astro-middleware-types?file=src/middleware.ts

Participation

  • I am willing to submit a pull request for this issue.
@Princesseuh
Copy link
Member

Even better than moduleResolution: 'node16' would be moduleResolution: 'bundler', but there's a few hiccups that prevents us from updating our templates at the moment:

  • Our language server doesn't handle node16 / bundler module resolution super well at the moment. It does work, but sometimes run into caching issues.
  • A lot of packages don't handle it super well due to packaging mistakes and other random reasons
  • It's a breaking change, so we need to wait until Astro 3.0 to do it

@raulfdm
Copy link
Contributor Author

raulfdm commented May 6, 2023

Hmmm... I see.

Then, what's the workaround? Set ourselves the resolution? Or should Astro provide types in a different strategy, such as having a central index.d.ts that references the other types??

@raulfdm
Copy link
Contributor Author

raulfdm commented May 7, 2023

I tested in my real project moduleResolution: 'node16', and I started having some issues with importing types from an internal package I have. Switching to moduleResolution: 'bundler' make everything works better.

According to the PR that introduces this module, bundler would be the best option for my case:
Screenshot 2023-05-07 at 07 24 42

microsoft/TypeScript#51669

@bluwy
Copy link
Member

bluwy commented May 7, 2023

Updating our typesVersion to map /middleware to /dist/core/middleware/index.d.ts should also fix it:

"typesVersions": {
"*": {
"app": [
"./dist/core/app/index"
],
"app/*": [
"./dist/core/app/*"
]
}
},

@raulfdm
Copy link
Contributor Author

raulfdm commented May 8, 2023

Thanks for the tip @bluwy . I opened a PR to apply this change and it fixed the problem indeed.

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 a pull request may close this issue.

3 participants