cmorten / opine

Minimalist web framework for Deno ported from ExpressJS.
https://github.com/cmorten/opine/blob/main/.github/API/api.md
MIT License
854 stars 43 forks source link

TypeScript Error: TS2345 [ERROR]: Argument of type 'string | URL' is not assignable to parameter of type 'string'. #42

Closed srepollock closed 4 years ago

srepollock commented 4 years ago

Issue

Setup:

(Also checked with the latest v0.18.0)

Details

When running my program:

// mod.ts
import {opine} from "https://deno.land/x/opine@main/mod.ts";

const app = opine();

app.get('/', (req, res) => {
    res.send('Hello Deno Land 🦕');
});

app.listen(3000);
console.log("running at htpss://localhost:3000");

with the command: deno run mod.ts

I get the following error:

error: TS2345 [ERROR]: Argument of type 'string | URL' is not assignable to parameter of type 'string'.
  Type 'URL' is not assignable to type 'string'.
  return new URL(url).pathname
                 ~~~
    at https://deno.land/std@0.60.0/path/win32.ts:917:18

TS2345 [ERROR]: Argument of type 'string | URL' is not assignable to parameter of type 'string'.
  Type 'URL' is not assignable to type 'string'.
  return new URL(url).pathname;
                 ~~~
    at https://deno.land/std@0.60.0/path/posix.ts:438:18

TS2345 [ERROR]: Argument of type 'string | URL' is not assignable to parameter of type 'string'.
  Type 'URL' is not assignable to type 'string'.
  return new URL(url).pathname
                 ~~~
    at https://deno.land/std@0.58.0/path/win32.ts:917:18

TS2345 [ERROR]: Argument of type 'string | URL' is not assignable to parameter of type 'string'.
  Type 'URL' is not assignable to type 'string'.
  return new URL(url).pathname;
                 ~~~
    at https://deno.land/std@0.58.0/path/posix.ts:438:18

TS2345 [ERROR]: Argument of type 'ParsedURL' is not assignable to parameter of type 'string'.
    const loc = encodeUrl(new URL(originalUrl).toString());
                                  ~~~~~~~~~~~
    at https://deno.land/x/opine@main/src/middleware/serveStatic.ts:227:35

Found 5 errors.

I've traced the issue to these two lines:

https://github.com/asos-craigmorten/opine/blob/2295ba25be8320e0a3adfb5fe9d53477b02d1650/src/middleware/serveStatic.ts#L227

https://github.com/asos-craigmorten/opine/blob/2295ba25be8320e0a3adfb5fe9d53477b02d1650/src/utils/parseUrl.ts#L70

I believe this is an error in type and should be addressed. I cannot run my program at this time. Let me know if you agree this is the issue and I can try to implement a fix.

FaberVitale commented 4 years ago

It's a deno bug. See https://github.com/asos-craigmorten/opine/issues/42#issuecomment-658159231

var myUrl = new URL(new URL("http://www.google.com/cookies"));

// TS2345 [ERROR]: Argument of type 'URL' is not assignable to parameter of type 'string'.
var myPathname = myUrl.pathname;

These 2 lines of code work in FF(latest), node 12 and chrome ( 83.0.4103.116 ) but do not work in deno:

deno --version
deno 1.2.0
v8 8.5.216
typescript 3.9.2
asos-craigmorten commented 4 years ago

Thanks for raising this @srepollock!

Having followed the issue on GitHub and Discord, and reading the changelog I still managed to overlook this 🤦

Relevant issues / PRs:

It's annoying that it deviates from what browsers / node allow, but the Deno team are aiming to write to WHATWG spec.

Should be simple enough to fix, we just need to cast to string as the URL constructor no longer accepts a URL as the first parameter.

Bidek56 commented 4 years ago

Did theses PRs make it into the 1.2.0?

srepollock commented 4 years ago

@Bidek56 It looks like it was from commit https://github.com/denoland/deno/commit/5c43131be1a36671fd564b82709e11dff66488f3

KaKi87 commented 4 years ago

Hello, I'm experiencing this issue using Opine 0.25.0 on Deno 1.4.2 ? Thanks

asos-craigmorten commented 4 years ago

Hey @KaKi87 can you provide a reproduceable example and the exact errors your are getting on the console? 😄

Update:

I've just created a file:

// example.ts
import { opine } from "https://deno.land/x/opine@0.25.0/mod.ts";

const app = opine();

app.use((req, res) => {
  res.send("Hello World");
});

app.listen(3000, () => console.log("Starting server on port 3000"));

Based on the readme, and ran with:

deno run --allow-net --allow-read --reload example.ts  

And worked fine for me for Deno 1.4.2:

$ deno --version 
deno 1.4.2
v8 8.7.75
typescript 4.0.3

So will need more information in order to diagnose! If you could open a new issue that would be appreciated.

KaKi87 commented 4 years ago

Well, turns out I was confused while reading the dependency tree from the deno info command, yes it is the exact same error, but no it is not comming from opine. Actually, it is coming from argon2, and if I would have cared to properly read the README, I would have figured out from the beginning that it was not supposed to be compatible with Deno versions after 1.2.3... Now I have to figure out how am I going to hash passwords. Thanks, anyway, and I'm truly sorry for the time you wasted because of me.