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

app: address some console errors #942

Merged

Conversation

fabiosantoscode
Copy link
Contributor

@fabiosantoscode fabiosantoscode commented Jan 23, 2020

The errors about the client pinging but the page not existing can only occur on the dev server. It shouldn't be running since npm start is setting the NODE_ENV to production, but still I added the NODE_ENV environment variable to app.json just to make sure heroku isn't running server.js raw.

Updated yarn.lock


Disregard the recommendations below if you use Edit on GitHub button to improve the docs in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This enables GitHub to link the PR to the corresponding bug and close it automatically when PR is merged.

Thank you for the contribution - we'll try to review and merge it as soon as possible. 🙏

Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it looks like we don't need this right? Could you check if Heroku sets env properly?

@fabiosantoscode
Copy link
Contributor Author

@shcheklein I did some digging yesterday before opening this PR, and I couldn't find any literature on Heroku which said it would run server.js raw, my setting the NODE_ENV variable is more like a last ditch effort.

@shcheklein
Copy link
Member

@fabiosantoscode
Copy link
Contributor Author

Didn't see that one! I'll revert that part then.

@shcheklein
Copy link
Member

@fabiosantoscode then we don't have anything else left on this PR, right? If ticket is not reproducible can we close it @jorgeorpinel ?

@fabiosantoscode
Copy link
Contributor Author

Just the yarn.lock update @shcheklein

@shcheklein
Copy link
Member

Do you know why does it remove so much stuff in yarn? Also let's remove the "Fixes ..." from the description and comment in the issue that you was not able to reproduce it?

@fabiosantoscode
Copy link
Contributor Author

That's right, it shouldn't close the issue at all. My bad.

@fabiosantoscode
Copy link
Contributor Author

No idea why it removes so many packages. Someone might have done some spring cleaning on their dependencies.

@shcheklein shcheklein temporarily deployed to dvc-landing-feature-893-2okhak January 24, 2020 21:18 Inactive
@shcheklein shcheklein merged commit 5842d4b into iterative:master Jan 24, 2020
@jorgeorpinel
Copy link
Contributor

We probably didn't run an update in a while.

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

Successfully merging this pull request may close these issues.

3 participants