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

TypeScript error: Property 'urlData' does not exist on type 'FastifyRequest'. #2

Closed
wong2 opened this issue Nov 1, 2017 · 13 comments
Closed

Comments

@wong2
Copy link
Contributor

wong2 commented Nov 1, 2017

In my route:

fastify.get('/', async (request, reply) => {
  const urlData = request.urlData()
  ...
})

TypeScript would complain:

src/app.ts(55,26): error TS2339: Property 'urlData' does not exist on type 'FastifyRequest'.

because there is no attribute urlData on FastifyRequest: https://github.com/fastify/fastify/blob/master/fastify.d.ts#L22

@jsumners
Copy link
Member

jsumners commented Nov 1, 2017

I really don't know what you want me to do with this.

@wong2
Copy link
Contributor Author

wong2 commented Nov 1, 2017

@jsumners sorry I should be more clear, updated

@jsumners
Copy link
Member

jsumners commented Nov 1, 2017

I understood what was happening. I just literally have no idea what you want me to do about it. This is a plugin. Therefore it isn't a part of the TS stuff in the core framework.

@wong2
Copy link
Contributor Author

wong2 commented Nov 1, 2017

@jsumners me neither. At a user's point of view, I'm starting my project with TypeScript + fastify, which works well, then I included this plugin, it stopped working, and there is no easy way to fix it. I think there should be some mechanism in fastify to support this, otherwise there would be more problem like this as more plugins are created.

@jsumners
Copy link
Member

jsumners commented Nov 1, 2017

cc: @fastify/fastify

This is why the TS things really don't belong in core. Unless you know of a fix @evanshortiss, the way I see it, the TS descriptions are just going to cause these sorts of problems. They should be something the user deliberately adds to their project.

@delvedor
Copy link
Member

delvedor commented Nov 1, 2017

Agree with @jsumners

@mcollina
Copy link
Member

mcollina commented Nov 1, 2017

I do not know exactly how to fix this and if typescript definitions can allow the level of usage that we need.

@mcollina
Copy link
Member

mcollina commented Nov 1, 2017

I think we should try to keep the TS definitions in, as they encourage more users to try our framework.

@wong2
Copy link
Contributor Author

wong2 commented Nov 1, 2017

No matter where the TS definitions is, there should be a way to solve this problem

@evanshortiss
Copy link
Member

I've opened #4 as an example of how this can be addressed - it should use urijs typings, but I only noticed after opening it, but using the core Node.js url.parse result conveys the idea for now. Thoughts?

Similar to core, it could be supported here or via DefinitelyTyped.

TS descriptions are just going to cause these sorts of problems. They should be something the user deliberately adds to their project.

Not sure I understand. The problem is the same regardless of where the d.ts files live - people will need them eventually. Ideally folks who need them will contribute back (yay!), but of course this is not going to always be the case (booo!).

Ideally folks who want to use TypeScript can open this kind of PR for plugins when required. Moving them to DefinitelyTyped might reduce noise if that is what is preferred? Folks will still open these kinds of issues though. I suppose if we don't support it then at least it can be closed with reasoning such as "typescript support is handled outside of the fastify org".

@evanshortiss
Copy link
Member

Ideally folks who need them will contribute back (yay!), but of course this is not going to always be the case (booo!)

I should point out, this comment is not meant to berate people who do not contribute. We all have many commitments in this life and time is not always available 👍

@jsumners
Copy link
Member

jsumners commented Nov 2, 2017

I suppose if we don't support it then at least it can be closed with reasoning such as "typescript support is handled outside of the fastify org".

^ that would be fine by me. The way I see it, people using Typescript are causing their own headaches. I don't need that headache propagated to me.

@evanshortiss
Copy link
Member

@jsumners it's a very short lived headache for an ultimately stronger codebase - in larger applications anyway. I've seen your disdain for TS repeatedly so I'm not going to bother preaching it's benefits to you! 😂

For small plugins like this it's should be fairly simple to CC someone on to take a look at the d.ts if you're pushing breaking changes.

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

No branches or pull requests

5 participants