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

Remove encoding package from dependencies #98

Open
MelkorNemesis opened this issue May 25, 2022 · 2 comments
Open

Remove encoding package from dependencies #98

MelkorNemesis opened this issue May 25, 2022 · 2 comments

Comments

@MelkorNemesis
Copy link
Contributor

MelkorNemesis commented May 25, 2022

before working on this, please check if the published version of the package @actions/github is more recent than the one shown below. If it isn't there's no chance this issue can be fixed

node-fetch version 2.6.7 has been causing us some esm/cjs compatibility issues. To avoid these issues we had to manually install encoding package as a dependency. Newer versions of node-fetch package no longer have encoding package as just a peerDependency (or dependency or devDependency) and when we learn that the application is using newer version of node-fetch we can remove the dependency from package.json.

$ npm ls node-fetch
[email protected]
└─┬ @actions/[email protected]
  └─┬ @octokit/[email protected]
    └─┬ @octokit/[email protected]
      └── [email protected]

More information about the issue:

@denes-fekeshazy
Copy link

Currently the application is still using @actions/[email protected], so we can not remove encoding as a dependency yet.
Also the problem is that while [email protected] does not list encoding as a dependency, it does trying to load the library in one of its functions. At build time it would produce the following code: module.exports = eval("require")("encoding");. Which is then failing the eval("require") check on this repo.

@simoneb
Copy link
Member

simoneb commented Jun 27, 2022

Correct. We have also put in place a test in CI which checks what ncc generates so that we know if the issue is still there. When we remove the dummy encoding dependency, if that test fails we know that the issue is still there.

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.

3 participants