amark / gun

An open source cybersecurity protocol for syncing decentralized graph data.
https://gun.eco/docs
Other
18.03k stars 1.16k forks source link

Heroku fixes #1243

Closed draeder closed 2 years ago

draeder commented 2 years ago

Removed 'stack' parameter to enable using the latest Heroku stack. This will allow new one-click deploys to use the latest Heroku stack, and avoid potential security issues.

draeder commented 2 years ago

@bmatusiak The prepare parameter is breaking Heroku deploys when NPM_CONFIG_PRODUCTION = true, so I have removed it. Re: https://github.com/amark/gun/pull/1237

bmatusiak commented 2 years ago

OK.. There is a fix for this. I remember it from another project. I just got to remember what it was

bmatusiak commented 2 years ago

3 different directions,

  1. need to remember to run unbuild before sending PRs ( this is ugly )
  2. need a unbuild ci to push unbuild like depend-abot during PR's ( this is ok but a lot more work after a pr merge)
  3. need to run unbuild on npm install (most dependable if run in prepare )

Heroku One Clickers should be running the latest unbuild. however in this state. its not.

amark commented 2 years ago

@draeder thanks for figuring this out.

@bmatusiak it is only because I have to tell Heroku to install additional packages (aws-sdk, etc.) that are NOT necessary for gun, I do this because I want to make sure THAT IF devs want to use S3 they ONLY EVER have to add their credentials via the logged in Heroku dashboard (versus forking GUN & hardcoding the keys - too easy to leak this way). However, I don't know any way to tell Heroku to install these deps automatically except for using some weird NPM subconfigs. These deps should NOT install when devs try out gun (in fact, NOTHING should go to a local dev machine except ws websockets, even NodeJS's webcrypto polyfill is/should be optional) though, they're opt-in and really only during prod or some tests.

Do you know a better way we can do this?

I think an automatic version of (1) is the best. I've been using prepublishOnly to unbuild automatically, but doing it per-commit makes more sense. Doing unbuild upon install... is my least favorite of the 3 options.

I need to accept this PR for now, as Heroku is (obviously) affected by a prepare/install process. Tho it is still weird to me it is crashing... I don't see why, tho I keep seeing them (and hearing reports), so I want to get that resolved meanwhile.

bmatusiak commented 2 years ago

so for option 1... we need to run a unbuild check to fail PR's with github ci workflow,

I know this is silly but, code sync is important, and should be handled immediately

however. it is weird. the unbuild script requires no dependencies.. and we don't really care if the "prepare" runs for the sake of the 1 click install 😵‍💫

amark commented 2 years ago

@bmatusiak is it easy to automate (1)? Cause yeah, agreed, code sync is ideal. Note tho: majority of contribs (like this one) are in standalone files already, really just GUN & SEA that are big-yet-small that need unbuild. So I don't think we should fail someone if an unbuild step doesn't happen, that sounds scary! More just like, when they call commit/push their machine is like "hey, we noticed you touched something that needs to be unbuilt, we're doing that now with your commit..." even if it doesn't show up in the commit itself thats probably fine as long as upon merge it automatically unbuilds then instead.

worldpeaceenginelabs commented 2 years ago

The merge helped! thx

https://donate-decentralize-ui.pages.dev/ if you click donate decentralize, deploy tracker, name it and set NPM_CONFIG_PRODUCTION true or false, it builds a running server from the amark/gun repo.

bmatusiak commented 2 years ago

@amark https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks pre-commit hooks.

create a hook to run unbuild-compare for Committing-Workflow Hooks show RED warning

need to see how to save hooks to repo

bmatusiak commented 2 years ago

@draeder well, I found the true fix for the Heroku 1 click deploy. its because it needs a buildpacks array in app.json like this

{
  "name": "gun-server",
  "website": "http://gun.eco/",
  "repository": "https://github.com/amark/gun",
  "logo": "https://avatars3.githubusercontent.com/u/8811914",
  "keywords": ["node", "gun", "gunDB", "database","graph","offline-first"],
  "description": "Javascript, Offline-First Javascript Graph Database Server Peer",
  "env": {
    "NPM_CONFIG_PRODUCTION": {
      "description": "If you do not want default features, set to \"true\".",
      "value": "false"
    },
    "PEERS": {
      "description": "Comma-separated list of peer urls to connect to",
      "required": false
    }
  },
  "buildpacks": [
    {
      "url": "heroku/nodejs"
    }
  ]
}

for some reason, NPM thinks it's necessary when using prepare

we know it auto-selects this same buildpack when running it.. but for 1 click, it doesn't during its first deploy build, instead it applies it on deploy start. and is why it's hard to catch