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

Dev Dep "smee-client" is included in distributed files #922

Closed
mrchief opened this issue Apr 26, 2019 · 9 comments

Comments

@mrchief
Copy link

commented Apr 26, 2019

Bug Report

Current Behavior
I'm trying to package an AWS lambda using webpack but run into this error:

Module not found: Error: Can't resolve 'smee-client'

Turns out lib/manifest-creation.js contains references to smee-client which is a dev-dependency.

image

Expected behavior/code
Dev Dependencies should not be included/referenced in the distributed files.

Environment

  • Probot version(s): 9.2.8
  • Node/npm version: Node 8/npm 5
  • OS: OSX 10.14.4

@issue-label-bot issue-label-bot bot added the bug 🐞 label Apr 26, 2019

@issue-label-bot

This comment has been minimized.

Copy link

commented Apr 26, 2019

Issue-Label Bot is automatically applying the label bug 🐞 to this issue, with a confidence of 0.95. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@welcome

This comment has been minimized.

Copy link

commented Apr 26, 2019

Thanks for opening this issue. A contributor should be by to give feedback soon. In the meantime, please check out the contributing guidelines and explore other ways you can get involved.

@mrchief mrchief changed the title Dev Dep "smee-client" are included in distributed files Dev Dep "smee-client" is included in distributed files Apr 26, 2019

@JasonEtco

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Hey @mrchief 👋 In the manifest creation flow, it attempts to connect to Smee. If the smee-client dependency isn't there, it'll just exit cleanly:

try {
// tslint:disable:no-var-requires
const SmeeClient = require('smee-client')
await this.updateEnv({ WEBHOOK_PROXY_URL: await SmeeClient.createChannel() })
} catch (err) {
// Smee is not available, so we'll just move on
// tslint:disable:no-console
console.warn('Unable to connect to smee.io, try restarting your server.')
}

What's probably happening for you is that your deployment is missing the environment variables that would otherwise skip this flow. This isn't a bug, because Probot isn't actually shipping smee-client - it's just using it if its there, when it needs to.

Does that make sense? Feel free to let me know if I can clarify this further!

@mrchief

This comment has been minimized.

Copy link
Author

commented May 3, 2019

Makes sense but what env vars should I be setting to make it skip?

@JasonEtco

This comment has been minimized.

Copy link
Member

commented May 3, 2019

All the ones that that flow would help you generate, APP_ID, WEBHOOK_SECRET. Also PRIVATE_KEY, but you may prefer other ways of including that!

@mrchief

This comment has been minimized.

Copy link
Author

commented May 3, 2019

I have those set but maybe they need to be set again within Webpack config... 🤔

@JasonEtco

This comment has been minimized.

Copy link
Member

commented May 3, 2019

From your screenshot, it looks like Webpack is transpiling the try/catch block away - that's likely causing it to throw an error that Probot would otherwise be handling. You can probably tweak your webpack settings to avoid that.

@mrchief

This comment has been minimized.

Copy link
Author

commented May 3, 2019

@JasonEtco I forgot about this. With Webpack, dynamic requires such as above become tricky and the solutions a bit nasty.

@stale

This comment has been minimized.

Copy link

commented Jul 2, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 2, 2019

@stale stale bot closed this Jul 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.