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

Module resolution doesn't work correctly when the module specifier includes double slashes // #25532

Closed
janispritzkau opened this issue Sep 9, 2024 · 6 comments · Fixed by #26920
Labels
bug Something isn't working correctly node compat

Comments

@janispritzkau
Copy link

janispritzkau commented Sep 9, 2024

Version: Deno 1.46.3

Deno REPL:

> import "npm:hono/streaming"
Uncaught TypeError: [ERR_MODULE_NOT_FOUND] Cannot find module 'file:///Users/x/Library/Caches/deno/npm/registry.npmjs.org/hono/4.5.11/dist/helper/utils/stream.js' imported from 'file:///Users/x/Library/Caches/deno/npm/registry.npmjs.org/hono/4.5.11/dist/helper/streaming//stream.js'
    at async <anonymous>:1:22

I'm not quite sure how this happens. I notice that there are two slashes in the "imported from" module.

@janispritzkau
Copy link
Author

janispritzkau commented Sep 9, 2024

It looks like ./ gets converted to .//index.js.

Source:

import type { Context } from '../../context'
import { TEXT_PLAIN } from '../../context'
import type { StreamingApi } from '../../utils/stream'
import { stream } from './'
// ...

Compiled:

import { TEXT_PLAIN } from "../../context.js";
import { stream } from ".//index.js";
// ...

@kt3k
Copy link
Member

kt3k commented Sep 11, 2024

Thanks for the report.

import { TEXT_PLAIN } from "../../context.js";
import { stream } from ".//index.js";
// ...

I confirmed this specifier ".//index.js" exists at /dist/helper/streaming/text.js in hono, and that seems causing the issue.

A more minimal reproduction is:

a.js

import "./b//c.js";

b/c.js

import "../d.js";

d.js

console.log("hi");

When executing a.js, Deno throws the below error:

$ deno a.js 
error: Module not found "file:///path/to/b/d.js".
    at file:///path/to/b//c.js:1:8

(Deno seems considering dir//file is 2 level deeper from dir without normalizing // part)

While Node executes the above without problem.

@kt3k
Copy link
Member

kt3k commented Sep 12, 2024

This looks fixed in Hono's side (honojs/hono#3405 )

@janispritzkau Can you try again with the latest version of Hono?

import "npm:[email protected]/streaming";

@marvinhagemeister
Copy link
Contributor

It's great that the hono team simplified the import, but we should be able to deal with ./ imports in npm packages regardless imo.

@janispritzkau
Copy link
Author

@kt3k Yes, that works. Thank you.

I also think this should be fixed on Deno's end. It seems like extra slashes being ignored in path resolution is a common behavior.

@kt3k kt3k changed the title Cannot find module npm:hono/streaming Module resolution doesn't work correctly when the module specifier includes double slashes // Sep 12, 2024
@kt3k
Copy link
Member

kt3k commented Sep 12, 2024

I agree that this should also be fixed on Deno's side. I updated the title to clarify the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
4 participants