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

atob is not defined when building with target: Node using webpack #59

Closed
thibaultdalban opened this issue Apr 2, 2020 · 17 comments
Closed
Labels
Type: Bug Something isn't working as documented, or is being fixed

Comments

@thibaultdalban
Copy link

Hello,

I'm facing this error when I try to request the API. My project uses the following packages:

  • "@octokit/auth-app": "^2.4.4"
  • "@octokit/rest": "^17.1.4"
  • "typescript": "^3.8.2"
import { createAppAuth } from "@octokit/auth-app";
import { Octokit } from "@octokit/rest"

const appOctokit = new Octokit({
  authStrategy: createAppAuth,
  auth: {
    id: APP_ID,
    privateKey: PRIVATE_KEY
  }
})
const { data } = await appOctokit.request("/app");
console.log(data)
"errorType": "ReferenceError",
"errorMessage": "atob is not defined",
"stack": [
    "ReferenceError: atob is not defined",
    "    at getDERfromPEM (/var/task/webpack:/myapp/node_modules/universal-github-app-jwt/dist-web/index.js:15:1)",
    "    at getToken (/var/task/webpack:/myapp/node_modules/universal-github-app-jwt/dist-web/index.js:50:1)",
    "    at githubAppJwt (/var/task/webpack:/myapp/node_modules/universal-github-app-jwt/dist-web/index.js:71:1)",
    "    at getAppAuthentication (/var/task/webpack:/myapp/node_modules/@octokit/auth-app/dist-web/index.js:8:37)",
    "    at getInstallationAuthentication (/var/task/webpack:/myapp/node_modules/@octokit/auth-app/dist-web/index.js:110:1)",
    "    at dist_web_hook (/var/task/webpack:/myapp/node_modules/@octokit/auth-app/dist-web/index.js:240:20)",
    ...
]

I tried to install the package atob-lite cause it was a dependency in the past https://github.com/octokit/rest.js/pull/1302, but it doesn't fix the issue.

@thibaultdalban thibaultdalban changed the title ato is not defined atob is not defined Apr 2, 2020
@gr2m
Copy link
Contributor

gr2m commented Apr 2, 2020

In what environment are you running your app? Server or browser?

@gr2m gr2m added the support label Apr 2, 2020
@gr2m
Copy link
Contributor

gr2m commented Apr 2, 2020

I wonder where universal-github-app-jwt is coming from, it's not a dependency of @octokit/auth-app.js?

@thibaultdalban
Copy link
Author

Hi @gr2m,

Thank you for your quick reply!

In what environment are you running your app? Server or browser?

My app is an AWS serverless Lambda running Node V12.16.1

I wonder where universal-github-app-jwt is coming from, it's not a dependency of @octokit/auth-app.js?

I'm not sure to understand the question, but universal-github-app-jwt has been introduced in @octokit/auth-app.js by #19

@thibaultdalban
Copy link
Author

It's strange; my app seems to be detected as running in a browser because before hitting this error, I've been requested to convert my private key using:

openssl pkcs8 -topk8 -inform PEM -outform PEM -nocrypt -in private-key.pem -out private-key-pkcs8.key

but the README indicates, Node handles both:

No conversation is needed in Node, both PKCS#1 and PKCS#8 format will work.

@thibaultdalban
Copy link
Author

This issue webpack/webpack#5756 could explain why my app is using the code under dist-web and not dist-node.

I tested this webpack/webpack#5756 (comment) and it loads the correct code, but it's not an option because it introduces potential side effects 😕

I tried to downgrade to "@octokit/auth-app": "^2.0.1" (so before introducing the browser compatibility) and I can execute the code without issue.

@gr2m
Copy link
Contributor

gr2m commented Apr 3, 2020

I'm not sure to understand the question, but universal-github-app-jwt has been introduced in @octokit/auth-app.js by #19

Sorry, I mixed up what repository I was in 🤦‍♂ please ignore my comment.

This is all a problem with creating universal libraries (and build tools), it's really a pain. There is no standard that everyone agrees on. We have a lengthy discussion here about a similar problem: gr2m/universal-user-agent#23 I hope that you find a workaround workaround there for you?

I hope that build tools add support for conditional exports, recently introduced to Node.js: https://nodejs.org/api/esm.html#esm_conditional_exports. This will be the right tool for the job, but until webpack and friends support it, it's going to be frustrating to all of us, I'm very sorry :(

@gr2m gr2m added Type: Bug Something isn't working as documented, or is being fixed and removed support labels Apr 3, 2020
@gr2m gr2m changed the title atob is not defined atob is not defined when building with target: Node using webpack Apr 3, 2020
@thibaultdalban
Copy link
Author

Thank you for the explanation and the links. I will take the time to read the discussion and test the different workarounds.
I very appreciated your help ⛑️ and your reactivity!
A big THANK YOU for all your contributions to the open-source community 👍

@brunopenso
Copy link

brunopenso commented Sep 4, 2020

HI, I have the same error here with AWS lambda and node 12.x and serverless framework.

package.json:
"@octokit/auth-app": "^2.4.15", "@octokit/core": "^3.1.2",
I solve the problem by commenting the line below from the serverless.yml

plugins:
  - serverless-offline
  - serverless-prune-plugin
#  - serverless-webpack
custom:
  prune:
    automatic: true
    number: 3

@apmitchell
Copy link

Thank you for the explanation and the links. I will take the time to read the discussion and test the different workarounds.
I very appreciated your help ⛑️ and your reactivity!
A big THANK YOU for all your contributions to the open-source community 👍

Did you ever find a workaround for this?

@gr2m
Copy link
Contributor

gr2m commented Apr 22, 2021

can we close the issue? The proper solution to this will be to implement the @octokit modules as native ES Modules and take advantage of conditional exports which are support by webpack, rollup, and esbuild now. But this is going to be a bigger project, there is nothing we can do in the meanwhile on our side to address the original problem of this issue. Use the suggested workarounds instead

@gr2m gr2m closed this as completed Apr 22, 2021
@thibaultdalban
Copy link
Author

thibaultdalban commented Apr 23, 2021

@gr2m yes I think we can close it. Also in recent nodejs release (v16) atob is expose as global nodejs/node#37786 so it probably fixes (in a way) the issue.

@joscha
Copy link

joscha commented May 7, 2021

For anyone else coming across this, the @octokit/auth-app package defines:
package_json

Now @rollup/node-resolve uses this default:

plugins_packages_node-resolve_at_master_·_rollup_plugins

which means that even when building for node it will pull dist-web/index.js.

You can fix it by teaching your rollup in the config to resolve to the main field first:

nodeResolve({
  mainFields: ['main', 'module']
}),

however this is a global setting, so you will change precedence for other modules as well.

@gr2m would it make sense to move the web dist to the browser field? E.g.:

  "source": "dist-src/index.js",
  "types": "dist-types/index.d.ts",
  "main": "dist-node/index.js",
  "module": "dist-node/index.js",
  "browser": "dist-web/index.js"

I think semantically it makes more sense?

@gr2m
Copy link
Contributor

gr2m commented May 7, 2021

I think semantically it makes more sense?

yes, unfortunately it's not that easy. The @pika/pack package we use is no longer maintained, we are looking into creating up our own build setup, which will export a proper ESM build, with conditional exports for Node, browsers, and deno, where necessary.

Eventually we will probably get rid of the build step altogether and just build native ES Modules, and move TypeScript definitions to .d.ts files. But will take a while

@joscha
Copy link

joscha commented May 8, 2021 via email

@gr2m
Copy link
Contributor

gr2m commented May 8, 2021

It’s not that easy. The browser build is the esm build. I don’t want to patch the setup we have with post build changes, I want to replace the whole build setup, it’s not an easy fix, unfortunately

@joscha
Copy link

joscha commented May 8, 2021 via email

@ScottChapman
Copy link

I'm stuck on Node v12 and am running into this building with webpack so I can deploy to IBM Cloud functions. My config is simple:

module.exports = {
  entry: {
    webhook: './actions/Webhook.js',
    probot: './actions/Probot.js',
  },
  target: 'node',
  output: {
    filename: '[name].js',
    path: path.resolve(__dirname, 'dist'),
  },
  optimization: {
    minimize: false
  }
};

Not sure what my options are?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented, or is being fixed
Projects
None yet
Development

No branches or pull requests

6 participants