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: this handling in static blocks results in undefined #7741

Closed
avaer opened this issue Jan 8, 2025 · 9 comments · May be fixed by #7334
Closed

🐛 BUG: this handling in static blocks results in undefined #7741

avaer opened this issue Jan 8, 2025 · 9 comments · May be fixed by #7334
Labels
bug Something that isn't working

Comments

@avaer
Copy link

avaer commented Jan 8, 2025

Which Cloudflare product(s) does this pertain to?

Wrangler

What versions are you using?

wrangler 3.100.0

What operating system and version are you using?

MacOS Sequoia

Please provide a link to a minimal reproduction

No response

Describe the Bug

The following minimal worker script doesn't seem to behave correctly wrt static blocks:

class lol {
  static {
    console.log('this 1', this);
  }
};

class lol2 {
  static a = 1;
  static {
    console.log('this 2', this);
  }
}

export default {};

In the browser and node, both this are defined, which seems correct.
However, in wrangler, the second this is undefined. Removing the static a = 1; seems to make a difference and results in the correct static block behavior.

Is this a specification implementation bug in the wrangler JS runtime?

Please provide any relevant error logs

⎔ Starting local server...
this 1 [class lol]
this 2 undefined
@avaer avaer added the bug Something that isn't working label Jan 8, 2025
@avaer
Copy link
Author

avaer commented Jan 8, 2025

I ran into this in the @discordjs/ws library.

@penalosa penalosa transferred this issue from cloudflare/workers-sdk Jan 9, 2025
@jasnell
Copy link
Member

jasnell commented Jan 9, 2025

Unable to reproduce the issue in workerd directly. Are you making use of wranglers transcoding mechanisms? For instance, are you using typescript or any kind of bundling?

Specifically, syntax like this is handled by v8, the same as node.js, so there should be no difference in behavior in the runtime and quick testing demonstrates that. However, I can imagine something in the transcoding process getting in the way... in which case this issue should be transfered over to the wrangler repo

@avaer
Copy link
Author

avaer commented Jan 11, 2025

Yes, I believe it is using the internal wrangler jsx bundling.

wrangler.toml:

name = "blah"
main = "main.jsx"
compatibility_date = "2024-09-23"
compatibility_flags = [ "nodejs_compat" ]
workers_dev = true

rules = [
  { type = "Text", globs = ["**/*.sql", "**/*.sql?raw"], fallthrough = true },
  { type = "Text", globs = ["**/*.txt", "**/*.txt?raw"], fallthrough = true },
  { type = "Text", globs = ["**/*.env", "**/*.env?raw"], fallthrough = true },
  { type = "Text", globs = ["**/*.cdc", "**/*.cdc?raw"], fallthrough = true },
  { type = "Text", globs = ["**/*.csv", "**/*.csv?raw"], fallthrough = true }
]

[[durable_objects.bindings]]
name = "AGENT"
class_name = "DurableObject"

[[migrations]]
tag = "v1"
new_classes = ["DurableObject"]

[vars]
WORKER_ENV = "production"

@jasnell
Copy link
Member

jasnell commented Jan 11, 2025

@dario-piotrowicz ... any ideas here?

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Jan 12, 2025

This is 100% a building issue

This can be clearly seen with wrangler dev by running the same worker with and without the --no-bundle flag:
Screenshot 2025-01-12 at 12 11 39

And it is actually an issue with esbuild (which we use for bundling) that I reproduced in node itself without workerd being involved at all:
https://github.com/dario-piotrowicz/esbuild17-static-init-blocks-bundling-bug-repro

The issue has actually already been fixed in esbuild (in 0.18.2) but unfortunately wrangler is stuck at 0.17.19 (see: #5222) so I think that this will be fixed whenever we can bump the esbuild version there (but I have no idea when, @penalosa might have an idea about that).

@dario-piotrowicz dario-piotrowicz transferred this issue from cloudflare/workerd Jan 12, 2025
@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Jan 12, 2025

@penalosa I've transferred the issue back to workers-sdk since as I mentioned in my comment I am pretty sure that this is a wrangler issue, I hope you don't mind, please let me know if you disagree 🙂

@penalosa
Copy link
Contributor

Thanks for diagnosing, @dario-piotrowicz! This will be fixed in the upcoming Wrangler v4, which you can try out by installing wrangler@next

@dario-piotrowicz
Copy link
Member

Thanks for diagnosing, @dario-piotrowicz! This will be fixed in the upcoming Wrangler v4, which you can try out by installing wrangler@next

Yay, that sounds great 😄

And yes I can confirm that the issue seem to be fixed in wrangler@next 🙂
Screenshot 2025-01-12 at 20 56 31

@dario-piotrowicz dario-piotrowicz linked a pull request Jan 13, 2025 that will close this issue
@penalosa
Copy link
Contributor

I'm going to close this—it was fixed for v4 in #6884, and will be released as part of Wrangler v4 soon. In the meantime, you can access this fix via wrangler@next

@github-project-automation github-project-automation bot moved this from Untriaged to Done in workers-sdk Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants