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

bug: deno test --doc incorrectly evaluates snippets #25718

Closed
iuioiua opened this issue Sep 19, 2024 · 3 comments · Fixed by #25720
Closed

bug: deno test --doc incorrectly evaluates snippets #25718

iuioiua opened this issue Sep 19, 2024 · 3 comments · Fixed by #25720

Comments

@iuioiua
Copy link
Contributor

iuioiua commented Sep 19, 2024

To reproduce:

  1. git clone https://github.com/denoland/std.git
  2. Run deno test --doc log/mod.ts using latest canary and get the following:
error: TS2304 [ERROR]: Cannot find name 'logger'.
    logger.debug(`running ${a} + ${b}`);
    ~~~~~~
    at file:///Users/asher/GitHub/std/log/mod.ts$121-131.ts:3:5

TS2304 [ERROR]: Cannot find name 'logger'.
    logger().debug(`running ${a} + ${b}`);
    ~~~~~~
    at file:///Users/asher/GitHub/std/log/mod.ts$78-95.ts:3:5

TS2304 [ERROR]: Cannot find name 'logger'.
    logger().debug(`running ${a} * ${b}`);
    ~~~~~~
    at file:///Users/asher/GitHub/std/log/mod.ts$78-95.ts:7:5

These same snippets run fine:

import { getLogger } from "@std/log";

const logger = getLogger("my-awesome-module");

export function sum(a: number, b: number) {
  logger.debug(`running ${a} + ${b}`); // no message will be logged, because getLogger() was called before log.setup()
  return a + b;
}
import { getLogger } from "@std/log";

function logger() {
  return getLogger("my-awesome-module");
}

export function sum(a: number, b: number) {
  logger().debug(`running ${a} + ${b}`);
  return a + b;
}

export function mult(a: number, b: number) {
  logger().debug(`running ${a} * ${b}`);
  return a * b;
}

Version: Deno 2.0.0-rc.3+c3bc692

@magurotuna
Copy link
Member

Here's why this issue happens - this code snippet

import { getLogger } from "@std/log";

const logger = getLogger("my-awesome-module");

export function sum(a: number, b: number) {
  logger.debug(`running ${a} + ${b}`); // no message will be logged, because getLogger() was called before log.setup()
  return a + b;
}

is first virtually converted into

import { getLogger } from "@std/log";

Deno.test("<test-name>", async () => {
  const logger = getLogger("my-awesome-module");

  export function sum(a: number, b: number) {
    logger.debug(`running ${a} + ${b}`); // no message will be logged, because getLogger() was called before log.setup()
    return a + b;
  }
});

but right after this, the code converter (precisely speaking, swc) detects that export function is not on the top level scope, which violates the spec. So this ends up being turned into:

import { getLogger } from "@std/log";

Deno.test("<test-name>", async () => {
  const logger = getLogger("my-awesome-module");
});

export function sum(a: number, b: number) {
  logger.debug(`running ${a} + ${b}`); // no message will be logged, because getLogger() was called before log.setup()
  return a + b;
}

Therefore logger unintentionally becomes an undeclared variable in the body of sum function.

I'll try to consider what would be the best fix for this.

@iuioiua
Copy link
Contributor Author

iuioiua commented Sep 19, 2024

I suggest printing a warning for a snippet containing exported symbols. I don't think snippets would typically contain them. In the meantime, I'll remove the export keywords from std.

@magurotuna
Copy link
Member

That sounds like a sensible solution, though I had started with the difference approach which can be seen #2570. The idea is to remove export to allow items annotated with export can be put inside Deno.test scope. What do you think about this approach?

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.

2 participants