Unexpected token on class private field check with in operator

[nicole0707]

Bug Description

The issue arises from undici@6.7.0, which is believed to be caused by the PR https://github.com/nodejs/undici/pull/2743/files#diff-7d87ea2cf8b3c20e25ac9a785c4ae7603f4a8c3d7bf3a80f5fb46265f0dba078R776

 ⨯ ./node_modules/undici/lib/web/fetch/util.js
Module parse failed: Unexpected token (608:63)
|             // 5. If object is not a default iterator object for interface,
|             //    then throw a TypeError.
>             if (typeof this !== "object" || this === null || !(#target in this)) {
|                 throw new TypeError(`'next' called on an object that does not implement interface ${name} Iterator.`);
|             }

Import trace for requested module:
./node_modules/undici/lib/web/fetch/util.js
./node_modules/undici/lib/web/fetch/index.js
./node_modules/undici/index.js

Reproducible By

The issue can be replicated in any Next.js project by simply upgrading undici to version 6.7.0

Expected Behavior

Run without error

Logs & Screenshots

Environment

Additional context

[Uzlopak]

Please provide a minimal viable code example.

[Uzlopak]

@tsctx

What if we dont use private attributes in FastIterableIterator? Then we could replace #target in this with 'target' in this

[tsctx]

Duplicate of #2923

[tsctx]

This is valid syntax. It is odd that it produces an error.

[Uzlopak]

Then it is an issue in next.js and should be reported there.

[nicole0707]

Could we use this.#target === undefined to check for undefined value? Especially the value will be used in subsequent lines this.#target[kInternalIterator]

      if (typeof this !== 'object' || this === null || this.#target === undefined) {
        throw new TypeError(
          `'next' called on an object that does not implement interface ${name} Iterator.`
        )
      }

      // 6. Let index be object’s index.
      // 7. Let kind be object’s kind.
      // 8. Let values be object’s target's value pairs to iterate over.
      const index = this.#index
      const values = this.#target[kInternalIterator]

FYI it's working locally and the issue seems resolved.

[tsctx]

If you cannot reproduce the problem without Next.js, then it is a problem with next.js.

[Uzlopak]

#target in this is not the same as !this.#target. Also there is a high chance, that next.js delays fixing the bug, if all by this bug effected apply workarounds.

[nicole0707]

I was trying swc and babel with next.js, they are both complaining.

[tsctx]

This workaround is a violation of the specification because it throws an error with a false value.

[nicole0707]

typeof this !== 'object' || this === null || this.#target === undefined has passed all unit tests, is there other workarounds?🙏

[tsctx]

I can add a workaround that conforms to the spec, but I don't want to add it because it would lead to performance regression.

[tsctx]

typeof this !== 'object' || this === null || this.#target === undefined has passed all unit tests, is there other workarounds?🙏

class A {
  next () {
    let error = false
    try {
      if (typeof this !== 'object' || this === null) {
        error = true
      } else {
        this.#target
      }
    } catch (e) {
      error = true
    }
    if (error) {
      throw new TypeError(
        `'next' called on an object that does not implement interface ${name} Iterator.`
      )
    }
  }
}

[nicole0707]

Thanks @tsctx, really appreciate all the effort you guys put into it. I just verified the change locally and it's working well.

Any chance we could get a bit more info on the perf impact?

[tsctx]

This is not a fundamental solution. Please report it Next.js and have the problem fixed.

Any chance we could get a bit more info on the perf impact?

Increasing the number of variables and adding try statements leads to extra overhead.

These are already slow and do not want to add even this small change.

[naftalimurgor]

If you cannot reproduce the problem without Next.js, then it is a problem with next.js.

Valid point seems like an issue with Nextjs and the clear context should be provided on how undici is used since Nextjs has a lot of moving parts as compared to working with Node plainly.

[nicole0707]

Fair enough, I will provide a minimal viable code example tomorrow.

[tsctx]

Fair enough, I will provide a minimal viable code example tomorrow.

There is no need to do so. This is because this is a problem with the Next.js parser, not with undici.

[tsctx]

> $ yarn build
   ▲ Next.js 14.1.3

   Creating an optimized production build ...
Failed to compile.

./app/layout.tsx
Module parse failed: Unexpected token (11:12)
File was processed with these loaders:
 * ./node_modules/next/dist/build/webpack/loaders/next-flight-loader/index.js
 * ./node_modules/next/dist/build/webpack/loaders/next-swc-loader.js
You may need an additional loader to handle the result of these loaders.
|     #a;
|     a() {
>         if (#a in this) {
|         // do noting
|         }

Import trace for requested module:
./app/layout.tsx


> Build failed because of webpack errors

https://github.com/tsctx/nextjs-private-field-in-operator-bug

[naftalimurgor]

> $ yarn build
   ▲ Next.js 14.1.3

   Creating an optimized production build ...
Failed to compile.

./app/layout.tsx
Module parse failed: Unexpected token (11:12)
File was processed with these loaders:
 * ./node_modules/next/dist/build/webpack/loaders/next-flight-loader/index.js
 * ./node_modules/next/dist/build/webpack/loaders/next-swc-loader.js
You may need an additional loader to handle the result of these loaders.
|     #a;
|     a() {
>         if (#a in this) {
|         // do noting
|         }

Import trace for requested module:
./app/layout.tsx


> Build failed because of webpack errors

https://github.com/tsctx/nextjs-private-field-in-operator-bug

Definitely a problem with Next.js parser

[nicole0707]

Yep, I'm hitting the same error too. I'll flag this with the Next.js folks and dig into it. It's holding us up on updating undici for now. Thanks for the help! I'll post any updates here and avoid any duplicate issues popping up.

[fersan96]

@nicole0707 Hello, i hope you're doing well, i would like to know what happend with this problem , is solved?

[HelaGone]

@nicole0707 did you reach to the people at Next? can you provide the link issue here please

[nicole0707]

Hey @fersan96 @HelaGone.

swc in Next.js doesn't fully support the transformation of private properties and methods. I am using a temporary solution by integrating babel-loader with required plugins. Given that transforming code can slow down building times, only undici packages is included by the loader.

A small change for webpack config in next.config.js like below

    webpack: (config, { isServer }) => {
      config.module.rules.push({
        test: /\.(?:js|ts)$/,
        include: [/node_modules\/(undici)/],
        use: [
          {
            loader: "babel-loader",
            options: {
              presets: ["next/babel"],
              plugins: [
                "@babel/plugin-transform-private-property-in-object",
                "@babel/plugin-transform-private-methods",
              ],
            },
          },
        ],
      });
      return config;
    },
  };

Also I reported these plugins to Next.js vercel/next.js#30174 (comment), feel free to leave a comment and probably could speed up that.