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

[docs, feature]: Feedback in general #18

Open
jsjoeio opened this issue Sep 15, 2019 · 12 comments
Open

[docs, feature]: Feedback in general #18

jsjoeio opened this issue Sep 15, 2019 · 12 comments

Comments

@jsjoeio
Copy link
Contributor

jsjoeio commented Sep 15, 2019

I think the setup instructions could be clarified to make it easier for first-time users. For instance, following the instructions, I get this after starting the app:

image

I know this is because the server isn't running and hasn't generated the queries but ideally, the setup should walk me through the entire setup (including the Relay part) before telling me to start the app.

Another thing that isn't clear is if I need to use two separate OneGraph apps? I am guessing no, but in the Setup part, you say use your "app's id". But then in the Relay part you say create a new app. I think we could clarify this section to avoid any confusion.

Also, I think we should try to solidify the use of env variables. There's a .env but then in the relay part, it tells me to add them inline with the yarn relay --watch command. It would be nice if I just added them to the .env.

While all of these may sound "negative", I do think this project is awesome and I know it's still very early on, but I did get things running :)

image

Last thing, I don't know the best solution, but the "issue" is having all the types of deployments in this one example. Now, to deploy to Netlify, I have to go delete the firebase, fly, now, files. Maybe instead you could do what services like Next.js and Nest.js do where you have a directory with examples:

Also, not your fault, but Netlify failed with I tried to deploy :( Something happened with the authorization? I ran yarn deploy:netlify, it opened a tab in my browser, I logged in and gave it permission then went back and reran the command but it failed saying unauthorized. Will figure this out. But might be worth noting in the docs.

image

Hmm...tried to deploy by connecting site to Netlify and it didn't work.
image

Looks like something went wrong with my functions maybe:
image

I'll have to investigate a bit further but I'm sure I can figure it out.

Summary of Feedback

Doc fixes

  • Rewrite Setup instructions to be more hold user's hand more
  • Decide on consistent user of environment variables
  • Refactor the main example and create /examples directory

Bugs

  • couldn't deploy to Netlify from command line (bug with Netlify?)

Feature Requests

  • OneGraph OneBlog CLI tool to generate boiler plate. It would ask what service you want to use (i.e. Firebase, now, Netlify), and then you wouldn't have all the unnecessary files

P.S. - this is the first time I've heard of Grommet. Looks pretty nice! 👌🏼

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

Hmm...just an idea here but I think we need to update the netlify.toml and add yarn build:netlify-functions. That got me one step further.

image

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

Also, another bug. For some reason, now is picking up my project, but I never told it to...
image

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

Annnnnd....now it's working!!! Had to add the domain to my OneGraph CORS Origins section, so glad that was mentioned in the docs. Made it easy to fix.

image

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

And last comment hopefully 😂 But got my own domain working too: https://blog.jsjoe.io/

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

Also, I noticed a hard-coded app id in the persistQuery file. Is that intentional?

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

So everything works, except for the "blogging" or persisted queries (I think). It's still pulling the "blog posts" from the original repo, instead of my new repo.

I'm guessing this is because I missed a step or am doing something wrong. Will investigate further though.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

Wow🤦🏼‍♂️

I didn't realize I had to change the query in the App... @sgrove gave me a quick intro to this, so I thought it "just worked".

image

Hmm...I'm wondering if it would be a good idea to use environment variables for the name and owner for that posts query. Or there could be a better idea. Not sure, but that should definitely be in the setup.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 15, 2019

Everything worked when I modified the postsRootQuery but as soon as I started changing other queries, the relay compiler started complaining.

[{"code":400,"message":"Invalid access_token, app_id for token does not match app_id of request"}]

I tried adding the @persistedQueryConfiguration to the mutations Post_AddReactionMutation and the Post_RemoveReactionMutation but it seems the problem might be something else.

It's strange, every time I run the relay command, I get different results, which is making it hard to debug.

relay-errors

Repo to reproduce errors: https://github.com/jsjoeio/blog.jsjoe.io.

Steps to reproduce:

  1. Run yarn relay --watch
  2. Observe errors

@dwwoelfel
Copy link
Collaborator

dwwoelfel commented Sep 16, 2019

This is great feedback, thanks @jsjoeio!

Working on fixes for the problems you encountered.

  • Reorder steps in readme so that you don't see "Failed to compile" when you run yarn start
  • Better way to handle persist query env vars (maybe .env?)
  • Clarify "new app" in readme
  • Put netlify build functions in netlify.toml *
  • Clarify hard-coded app id in persistQuery file
  • Move repo name and owner to fixedVariables
  • Slug in url
  • Investigate Gatsby

* "command" in netlify.toml didn't seem to do anything for me, so I put all netlify steps the deploy:netlify script.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 16, 2019

No problem! Sweet 😄

Last two things I'll say:

  • for SEO, would be nice if the routes for each "post" used a slugified version of the issue title rather than simply the issue number.
  • could totally see people wanting to use this "blog setup" but with Gatsby instead of Razzle. Might be an interesting side venture of this idea.

@dwwoelfel
Copy link
Collaborator

dwwoelfel commented Sep 16, 2019

@jsjoeio made updates that resolve most of the problems you encountered.

Biggest changes are

  • the repo name and owner are now provided via "fixedVariables" set in the .env file. If you pull the latest changes into your fork, add REPOSITORY_FIXED_VARIABLES='{"repoName": "blog.jsjoe.io", "repoOwner": "jsjoeio"}' to the .env file

  • The url includes the post title

  • The deploy:netlify command builds everything needed to deploy.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 17, 2019

@dwwoelfel Awesome! Thanks so much!

Pulled in all the changes and everything looks to be working smoothly!
WtNEMowaAH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants