Skip to content

Conversation

@valarnin
Copy link
Collaborator

proxy-agent was a "breaking" change that required a minor code tweak. For some reason, vscode's eslint display shows an error for that line, but the actual npm run lint doesn't 🤷

@github-actions github-actions bot added util /util needs-review Awaiting review labels Nov 24, 2024
@github-actions github-actions bot removed the needs-review Awaiting review label Nov 25, 2024
@wexxlee
Copy link
Collaborator

wexxlee commented Nov 25, 2024

I haven't dug into the failed checks, other than seeing webpack is having issues with this. Saw this was approved though, so just wanted to make sure we held off on merging.

@valarnin
Copy link
Collaborator Author

Yeah, I won't merge until the issue is fixed. Just ran out of time to debug it.

@valarnin
Copy link
Collaborator Author

valarnin commented Dec 6, 2024

This latest commit should hopefully fix the webpack issues. It doesn't really feel like a "good" fix, but I'm really not sure what package update caused the break to begin with.

Basically, webpack stopped using the ts-node/esm loader by default for the manifest loader, and also even after forcing it to use the correct loader, the generated cjs source wasn't loading properly in loader-runner, some extra debug code in node_modules/loader-runner/lib/loadLoader.js showed that the imported module's function path was module.default.default instead of module.default.

Maybe @MaikoTan has some ideas?

@MaikoTan
Copy link
Collaborator

MaikoTan commented Dec 6, 2024

Maybe @MaikoTan has some ideas?

I am not sure...

I noticed the webpack was upgraded from 5.76 to 5.92 in this PR, and I could use the 5.76 version to build without the --loader option, maybe there are some changes between these versions that cause the issue?
But on the other hand, according to the webpack's official website, the --loader method was currently a recommended way.
And I am tired to deal with webpack and want to switch to Vite or Rollup now 😭 ....

@xiashtra xiashtra mentioned this pull request Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

util /util

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants