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

FIX deploy with-firebase-hosting #3946

Merged
merged 9 commits into from Mar 8, 2018
Merged

FIX deploy with-firebase-hosting #3946

merged 9 commits into from Mar 8, 2018

Conversation

sarovin
Copy link
Contributor

@sarovin sarovin commented Mar 6, 2018

I have this error with yarn deploy:

There was an unknown problem while trying to parse function triggers. Please ensure you are using Node.js v6 or greater.

@jthegedus can you test this fix?

@@ -7,7 +7,8 @@
"dev": "next src/app",
"serve":
"echo \"for details see:\n\thttps://github.com/firebase/firebase-tools/issues/535 \n\thttps://github.com/zeit/next.js/issues/3167\";",
"deploy": "firebase deploy",
"predeploy": "mkdir -p \"dist/functions\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't need this as build-funcs will create this when run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we must run firebase deploy in dist/functions for fix the problem. If i run yarn clean and after cd \"dist/functions\" && firebase deploy, we have an error because dist/functions not exists.

Copy link
Contributor

@jthegedus jthegedus Mar 8, 2018

Choose a reason for hiding this comment

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

cd "dist/functions" && firebase deploy

It errors because after you cd into dist/functions firebase deploy tries to deploy a directory called dist/functions which doesn't exist from the relative directory of dist/functions .

firebase deploy knows to deploy dist/functions from https://github.com/zeit/next.js/pull/3946/files#diff-0bde36a03f5abc11a925591823b9d3c2R13

@@ -7,7 +7,8 @@
"dev": "next src/app",
"serve":
"echo \"for details see:\n\thttps://github.com/firebase/firebase-tools/issues/535 \n\thttps://github.com/zeit/next.js/issues/3167\";",
"deploy": "firebase deploy",
"predeploy": "mkdir -p \"dist/functions\"",
"deploy": "cd \"dist/functions\" && firebase deploy",
Copy link
Contributor

@jthegedus jthegedus Mar 6, 2018

Choose a reason for hiding this comment

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

and this shouldn't be required either because https://github.com/zeit/next.js/blob/76582b8e437dadc4dce18ec6ee525acb6f0d2cdb/examples/with-firebase-hosting/firebase.json#L13 should be picked up on firebase deploy.

And moving dir might mess up the deployment step of any hosting or db parts of the deployment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why...but this doesn't work for me...

@jthegedus
Copy link
Contributor

jthegedus commented Mar 6, 2018

In the last update I added Firebase Deployment Hooks https://github.com/zeit/next.js/blob/76582b8e437dadc4dce18ec6ee525acb6f0d2cdb/examples/with-firebase-hosting/firebase.json#L10 and https://github.com/zeit/next.js/blob/76582b8e437dadc4dce18ec6ee525acb6f0d2cdb/examples/with-firebase-hosting/firebase.json#L15 so you shouldn't need to create any folders as they are created on the output of the build-funcs, build-app & build-public steps.

@sarovin could you please double check and post your Node version, the version of firebase-tools and the full stack trace? Cheers

@jthegedus
Copy link
Contributor

jthegedus commented Mar 6, 2018

@sarovin Oh, I think the issue may be because I didn't change the predeploy hooks to use npm. I use Yarn in my own personal examples and the examples here use npm.

Could you try

  1. reverting your changes
  2. change
    https://github.com/zeit/next.js/blob/76582b8e437dadc4dce18ec6ee525acb6f0d2cdb/examples/with-firebase-hosting/firebase.json#L10
    and
    https://github.com/zeit/next.js/blob/76582b8e437dadc4dce18ec6ee525acb6f0d2cdb/examples/with-firebase-hosting/firebase.json#L15

to use npm run instead of yarn.

  1. try running it again

But also still post your Node version, firebase-tools version and the full stack trace. And are you using Yarn or npm? Because you should only use npm with this example. If you want a Yarn example then please see https://github.com/jthegedus/firebase-functions-next-example

@sarovin
Copy link
Contributor Author

sarovin commented Mar 6, 2018

@jthegedus yes, with npm run i have the same problem.

Versions:
yarn: 1.5.1
node: 8.9.3
firebase-tools: 3.17.4

@jthegedus
Copy link
Contributor

jthegedus commented Mar 6, 2018

I'll see if I can reproduce this problem tonight (12hrs from now). I have not come across it with my Yarn example which would indicate it's something to do with npm and the scripts.

@jthegedus
Copy link
Contributor

jthegedus commented Mar 7, 2018

I just reverted your changes, updated the predeploy scripts in firebase.json to use npm run in place of yarn and it all works as expected for me (no permissions to push the fixes to your fork).

Node: 8.9.4
npm: 5.6.0
firebase-tools: 3.17.4

The deployed code does fail, but that is as expected #3886 (comment) until a new Next.js release with that fix.

Just to be clear, this example should be run with npm alone. Yarn and npm should not be mixed.

Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

The changes as per the above comment should be made.

@sarovin
Copy link
Contributor Author

sarovin commented Mar 7, 2018

Thanks @jthegedus. I have added you as collaborator to my fork ( if you want make changes ).

@jthegedus
Copy link
Contributor

@sarovin Try running npm run deploy from the root of this project now, it should work.

Unfortunately, I've not been able to reproduce your error. I have however tested the versions 5.0.0, 5.0.1.canary.10, 5.0.1.canary.11 & 5.0.1.canary.13 releases with npm & yarn and have found that versions canary.11 onwards fix any issues regarding _ files, but again, I cannot reproduce your error.

@timneutkens timneutkens merged commit d351824 into vercel:canary Mar 8, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants