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

ReferenceError: e is not defined error occurs when running next dev --turbo #72232

Open
y-hsgw opened this issue Nov 3, 2024 · 5 comments
Open
Labels
bug Issue was opened via the bug report template. linear: turbopack Confirmed issue that is tracked by the Turbopack team. Turbopack Related to Turbopack with Next.js.

Comments

@y-hsgw
Copy link

y-hsgw commented Nov 3, 2024

Link to the code that reproduces this issue

https://github.com/y-hsgw/with-urql

To Reproduce

When using next dev --turbo with a Next.js application, the following error message appears:

ReferenceError: e is not defined
    at Module.Kind (/path/to/.next/server/chunks/ssr/node_modules_xyz.js:1376:18)
    at Kind (turbopack://[project]/node_modules/@urql/core/src/gql.ts:84:30)
    ...

The application works without issues when running the next dev command.

Current vs. Expected behavior

bug screenshot:

スクリーンショット 2024-11-03 12 36 54

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 24.0.0: Tue Sep 24 23:36:26 PDT 2024; root:xnu-11215.1.12~1/RELEASE_ARM64_T8103
  Available memory (MB): 8192
  Available CPU cores: 8
Binaries:
  Node: 22.10.0
  npm: 10.9.0
  Yarn: 4.5.1
  pnpm: N/A
Relevant Packages:
  next: 15.0.2 // Latest available version is detected (15.0.2).
  eslint-config-next: N/A
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.6.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Turbopack

Which stage(s) are affected? (Select all that apply)

next dev (local)

Additional context

I also submitted an issue to urql, but they responded that it was an issue on the Next.js side.

urql-graphql/urql#3704

@y-hsgw y-hsgw added the bug Issue was opened via the bug report template. label Nov 3, 2024
@github-actions github-actions bot added the Turbopack Related to Turbopack with Next.js. label Nov 3, 2024
@TheFabi8A
Copy link

In the 15.0.2 version --turbo command has replaced to --turbopack #71730

@y-hsgw
Copy link
Author

y-hsgw commented Nov 3, 2024

The same issue occurs with the --turbopack command as well.

@apostolos
Copy link
Contributor

I'm getting the same error with gql.tada.

I think the problem is related to @0no-co/graphql.web v1.0.10+ which urql is also using internally.

If I pin @0no-co/graphql.web to v1.0.9, it works fine.

One of the following PRs is introducing the problem:

@timneutkens timneutkens added the linear: turbopack Confirmed issue that is tracked by the Turbopack team. label Nov 4, 2024
@E3FxGaming
Copy link

Through bisection i figured out that it's a problem with

Specifically these five lines that introduce a setter cause the problem.

Removing those lines even in the current master branch HEAD of the /0no-co/graphql.web project and building the project results in a dist result that doesn't throw an error when used with tubopack.

I assume the broader problem is that the document function returns ast.DocumentNode, which in turn uses a custom Or type to express that it's either their own DocumentNode or a DocumentNode from graphql-js.

Both of those DocumentNode have an optional property readonly loc?: Location;, which the defined getter and setter don't refer to though, instead they work with this scoped variable loc from the document function.

The combination of getter and setter existing for an (optional) property that is otherwise never used, in combination witth the getter and setter not referring to the actual property of ast.DocumentNode is probably something Turbopack can't handle.

@E3FxGaming
Copy link

Alright, looked into this some more. I'll summarize my findings and then we can maybe discuss where this should be reported, since I don't believe it's something that Turbopack can fix/should attempt to fix.

I'll be referring to the following code snippets (all links are permanent links to the current head-commits on their respective master/main branch, as of writing this):

  • DocumentNode from graphql-js. This has gained a new property tokenCount yesterday which you can ignore for the purposes of this discussion. What's important is that it has 3 readonly fields, of which one, called loc, is optional.

  • DocumentNode from graphql.web is funamentally identical to the graphql-js, version. It's just a type definition that says DocumentNode can either be of the graphql-js variety or of the graphql.web variety.

  • DocumentNode from urql. Their type definition says DocumentNode is of the DocumentNode type from graphql.web, but not of the graphql-js type.

  • parse from qraphql.web. This can take a string and parse it into a DocumentNode. Notably it can take ParseOptions which can tell the next method mentioned below that the loc property should remain undefined on the DocumentNode.

  • document from graphql.web. This takes an input string and a boolean that specifies whether no location information should be added and constructs a DocumentNode.

  • stringifyDocument from urql. The premise of this method shouldn't introduce any problems: take a DocumentNode and turn it into a string. The problem that it introduces can be found in the method descriptions, which reads "When a DocumentNode is passed to this function, it caches its output by modifying the loc.source.body property on the GraphQL node.".

This IMHO is a flawed function, because it assumes there is a place in the DocumentNode where information can always be cached. It does a little checking with !node.loc but then throws all caution out the window with an assignment to a (node as any).loc cast. This initially worked fine: the !node.loc check correctly identified whether DocumentNode even has a loc and would block assignment if no such property existed.

It worked until it did not work anymore: This pull request added a getter to DocumentNode for the Ono-co/gql.tada GraphQL query engine, which got backported to graphql.web last week for compatibility reasons.

Having a getter for a potentially undefined property causes problems, especially if you use "property is accessible" as an indicator for "property is assignable". And so it crashed, resulting in this gql.tada issue, where the stacktrace indicates that it crashed exactly in stringifyDocument.

All of this of course completely ignores that loc was originally meant to be readonly. If it exists fine, you can alter the properties of Location assigned to loc, since those aren't readonly, but otherwise you probably shouldn't mess with caching information in an undefined variable.

Anyways, instead of overthinking this entire situation adding a setter was the chosen solution that worked for them, but that now just propagates the issue to turbopack users. Turbopack (rightfully) assumes readonly properties shouldn't have a setter and thus, during compilation, apparently discards that function. When it comes to putting the compilation result back together in the browser of the user, the "ReferenceError: e is not defined" error from the title of this issue gets thrown.

How I would fix this problem: roll back the introduction of the setter and getter and improve the stringify method in two ways:

  • improve the detection of whether loc is actually not undefined.
  • when loc is defined, alter the properties of the existing Location instead of assigning a new Location to the originally readonly property. This would get rid of the (node as any) cast too.

Something I have not fully understood yet is the entire graphql-tag situation described in the pull request that introduced the getter, but maybe they could fix the problem on their graphql-tag end in a different way that doesn't involve changes to gql.tada (and by extension graphql.web).

Overall there are A LOT of parties/people involved in this entire situation and turbopack users are simply on the receiving end of an unfortunate situation. Following the chain of dependencies I believe a good place to start would be reporting the problem to urql, who could then decide whether this is something they would want to improve. This time it's turbopack, but next time it could be some other dependency/project that runs into trouble with urql.

Reporting an issue to urql involves creating a reproduction, which I wouldn't mind doing but I first wanted to hear the opinions of other people here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. linear: turbopack Confirmed issue that is tracked by the Turbopack team. Turbopack Related to Turbopack with Next.js.
Projects
None yet
Development

No branches or pull requests

5 participants