-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Add gatsby-source-mongodb plugin to gatsby #1570
Conversation
Deploy preview ready! Built with commit aa719c7 |
Deploy preview ready! Built with commit aa719c7 |
Deploy preview ready! Built with commit aa719c7 |
Woah! Super cool, was hoping someone would do this :-D Adding some quick reviews inline. |
// it like the site has a built-in database constructed | ||
// from the fetched data that you can run queries against. | ||
|
||
// HnStory is a data node type created from the HN API “allHnStory” is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments need updated
examples/using-mongodb/package.json
Outdated
{ | ||
"name": "using-mongodb", | ||
"private": true, | ||
"description": "Gatsby example site using using-mongodb", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"using gatsby-source-mongodb"
examples/using-mongodb/src/html.js
Outdated
@@ -0,0 +1,53 @@ | |||
import React, { Component } from 'react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally you don't need a custom html.js. Just use https://www.gatsbyjs.org/packages/gatsby-plugin-react-helmet and then the react-helmet stuff will get added automatically to the <head>
@@ -0,0 +1,28 @@ | |||
import React from "react" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be layouts/index.js
const story = this.props.data.mongoDbDocField; | ||
|
||
return ( | ||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's some inconsistent formatting. We use Prettier in this repo. Just run npm run format
from the root of the repo to fix things up.
@@ -0,0 +1,36 @@ | |||
# gatsby-source-mongodb | |||
|
|||
Source plugin for pulling data into [Gatsby](https://github.com/gatsbyjs) from Wordpress sites using the [Wordpress JSON REST API](https://developer.wordpress.org/rest-api/reference/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update comments
collection: collectionName, | ||
internal: { | ||
mediaType: 'application/json', | ||
type: `mongoDBDocField`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The node type should be based on the collection. Otherwise there could be field type conflicts between fields in different collections e.g. one collection a field is a string and another it's a int.
Also since it looks like you're supporting connecting to multiple DBs which is a cool feature, perhaps have the type name be something like Mongo${DB_NAME}${COLLECTION_NAME}
.
You might want to make the collection
field a bit more unique to lessen the chance of conflicting with a field named "collection".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used mongodb{dbName}{collection}
I will look into the feedback this evening. Planning to incorporate the possibility to add children in the mongoDB document and a parent reference, if one is been provided use that otherwise use a default. |
Awesome! A few other things that might be nice:
|
Deploy preview failed. Built with commit d36ecc1 https://app.netlify.com/sites/using-contentful/deploys/59710409cf321c01f861a5d7 |
Deploy preview failed. Built with commit e04ce20 https://app.netlify.com/sites/using-styled-components/deploys/597794270752d047495f6a40 |
Deploy preview failed. Built with commit aa719c7 https://app.netlify.com/sites/using-remark/deploys/597a3bff7960b105249a1090 |
Deploy preview failed. Built with commit d36ecc1 https://app.netlify.com/sites/using-postcss-sass/deploys/59710408cf321c01f861a5c8 |
Deploy preview failed. Built with commit e04ce20 https://app.netlify.com/sites/using-glamor/deploys/597794260752d047495f6a3c |
Deploy preview failed. Built with commit d36ecc1 https://app.netlify.com/sites/image-processing/deploys/59710409cf321c01f861a5ce |
It'd be cool too to have the example site pointing to a mongodb instance in the cloud somewhere (I'm assuming one of the hosted MongoDB services have tiny free instances available). We build example sites on every PR as integration tests so we'd need this. Plus it'd be nice as someone checking out the plugin could run the example site without setting up a local mongo instance. |
Just merged master to stop the failing netlify builds which means you'll need to pull. |
@@ -0,0 +1,3 @@ | |||
/*.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also ignore tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok done
I need to work on the example site more into detail. This will be something for next week, I will indeed look into a free cloud mongoDB instance. |
Oh also in gatsby-remark-images as you committed a compiled test file there https://github.com/gatsbyjs/gatsby/pull/1570/files#diff-1a268099af2a7db53e390ac9c8d35862 That's what I saw before |
I isolated the mapping code ... In the mainwhile I delete the node_modules and reinstall it and do a gatsby-dev on the folder |
Hiya @jorishermans! 👋 This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here. Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! 💪💜 |
It contains:
Still need to write a fill mongodb script in the example site, that will make it easier to set it up when someone wants to test this out on his local machine.