junior-dev-struggle-bus / juniordevstrugglebus

Website for the Junior Dev Struggle Bus Meetup
https://www.juniordevstrugglebus.com
MIT License
9 stars 18 forks source link

Experimental Netlify function setup for eventual FaunaDB integration #41

Closed Xoadra closed 4 years ago

Xoadra commented 4 years ago

I have an operational Netlify function implemented successfully. It can be executed on the Resources page via the placeholder function button that I created on the add resources widget. This will simply modify the text to the right of the button and update it with output supplied by the called Netlify function.

While everything works just fine, there are some important caveats. If we want to move forward with this Netlify function and FaunaDB integration, it will likely change how the community will be able to continue contributing to the website.

You will notice that there is now a netlify.toml file that I've included in my PR. This is going to be important in order to allow others to contribute to developing Netlify functions for the website and in other community projects.

Additionally, simply running npm start locally will no longer be sufficient in order for people to work on their own separate forks of the site. With Netlify, we will now need to start using Netlify Dev in order to build, test, and use Netlify functions locally. This means making updates to our README.md file and properly fleshing out proper contributing guidelines and development instructions as is mentioned in issue #15.

Finally, it appears that using netlify dev may require access to the siteId environment variable in order to use Netlify functions locally. It also exposes access to netlify deploy which means contributors could accidentally or maliciously deploy to production from their local machines and potentially bypass the existing automated deployment process that is currently used. The siteId value is stored in a .netlify folder that's generated by using netlify link as is explained in detail here. By default, .netlify is hidden from git which suggests it should not be committed. We don't want to accidentally expose sensitive data on the website, so we may need to find a way to safely expose the siteId variable so that the community can continue to contribute to the site. We also don't want to allow other deploys without them going through our existing protocols, so we will need to make sure to block this off somehow.

I would recommend that whoever reviews this PR try to run it locally without having to use netlify link or the siteId variable as a sanity check. If you're able to get it to work without these two things, then perhaps this won't be an issue after all. If it does happen to be an issue, we will need to figure out a workaround for this problem.

One more thing: while the function I wrote works when called from the resources page, it does not appear to work when using netlify functions:invoke. Instead, the following error and stack trace is produced:

ran into an error invoking your function
{ FetchError: request to http://localhost:8888/.netlify/functions/resources failed, reason: connect ECONNREFUSED 127.0.0.1:8888
    at ClientRequest.<anonymous> (/Users/xoadra/.nvm/versions/node/v10.16.0/lib/node_modules/netlify-cli/node_modules/node-fetch/lib/index.js:1455:11)
    at ClientRequest.emit (events.js:198:13)
    at Socket.socketErrorListener (_http_client.js:392:9)
    at Socket.emit (events.js:198:13)
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
  message:
   'request to http://localhost:8888/.netlify/functions/resources failed, reason: connect ECONNREFUSED 127.0.0.1:8888',
  type: 'system',
  errno: 'ECONNREFUSED',
  code: 'ECONNREFUSED' }

You can read more about the netlify functions:invoke command via their docs if you'd like. While this doesn't appear to affect the function's usability on the website, it's good to be aware that this issue exists in the event it rears its head in another situation in the future.

Let me know if you run into any issues when running this code. Otherwise, I look forward to working on the next step with @nspilman!

nspilman commented 4 years ago

Looks good! I'll set up a Netlify instance for the dev branch so we can test before deploying to production.

nspilman commented 4 years ago

Next step is for me to update the function to call the db, yeah?

bhurstGH commented 4 years ago

Normally Netlify will build a Deploy preview for all PRs -- it may not be doing it here because the PR was to the dev branch and Netlify is watching master.

Just pointing that out since I noticed a Deploy preview wasn't built for this PR.

Xoadra commented 4 years ago

@nspilman Yeah, basically so we can add items to the database from it. That'll be next first step.

However, we will also need to revamp how resources are fetched once the database has been fully updated. Because of the limit that's placed on Netlify functions, we'll want to create a separate function for fetching data from the database.

Before we merge these changes back to master however, we will also need to add a new form for submitting resources in the first place. Netlify has an API for that that we'll want to use, but as to the other implementation details, we can discuss those at a later point.

Xoadra commented 4 years ago

For reference, this PR is for the eventual resolution of #30.

Since this portion of it is done, what remains is for @nspilman to finalize the FaunaDB integration with the Netlify function and for @Xoadra to create the new add resource form page that then posts form data to the Netlify function. Then we'll just need to add the existing resources to the new database and pull resource data from the database instead of the Google sheet into the Resources page.