Closed mars closed 5 years ago
@mars I ran into an issue that I believe I've resolved, but I want to double-check that I'm doing this right. I ran Heroku's @heroku/update-node-build-script
before finding this PR thread and it recommended I add "heroku-postbuild": "echo Skip build on Heroku"
to my package.json scripts. After adding that script and before updating to https://github.com/mars/create-react-app-buildpack#deploy-with-auto-build
, the build was fine (I was just using https://github.com/mars/create-react-app-buildpack
). Once I updated to https://github.com/mars/create-react-app-buildpack#deploy-with-auto-build
, the build did not execute the build
script in my package.json, unsurprisingly, because I now had heroku-postbuild
.
So long story short, when using the new buildpack that you will officially launch on March 11, users need to make sure they don't use the heroku-postbuild
script that is added by @heroku/update-node-build-script
. Is that correct?
Thanks @ajmueller 😸 I updated the PR description with that ⚠️ warning.
⚠️ If you already set
"heroku-postbuild": "echo Skip build on Heroku"
to avoid the new auto build behavior, then make sure to remove that property frompackage.json
so that auto build can run.
if I've already added "heroku-postbuild": "echo Skip build on Heroku"
/ "heroku-run-build-script": true
and don't change anything else, is it correct that my builds will break on March 11th? It seems like that is the case. I assume that each time Heroku builds it pulls the latest buildpack version, so when this PR is merged and things update upstream, this new behavior will take over.
pointing this out because I wonder how much visibility this PR has for people who may have seen the log in their Heroku builds to add "heroku-postbuild": "echo Skip build on Heroku"
but have not seen this update. Since this is not something you have to update to manually, there is no changelog visibility etc. I would be in that bucket had I not made #155 😛
also pointing this out because
No changes are required to apps deployed with the master version of this buildpack.
above is somewhat vague in terms of what changes are or are not required given previous actions taken re: the Heroku changes.
@ekilah I understand your concern, but this is not an official Heroku buildpack so there's no further visibility I can give the change 😔
I've already added
"heroku-postbuild": "echo Skip build on Heroku"
/"heroku-run-build-script": true
and don't change anything else, is it correct that my builds will break on March 11th?
If you add both of those things to your package.json
, they are conflicting with each other so I honestly have no idea what will happen.
If you add only the "Skip build on Heroku" thing to your app, then yes, it's up to you to remove that thing when it's no longer appropriate, on or after March 11.
I recommend adding only "heroku-run-build-script": true
and using this PR branch (per the PR description above) to ensure your app remains deployable during this transition on March 11. Even with this change, you'll need to remember to reset the buildpack back to the master branch or latest buildpack registry release.
Do not make any of these heroku-postbuild or heroku-run-build-script changes to your app, and simply avoid deploying on March 11th until the new platform behavior is launched and this PR is merged & released for this buildpack.
@mars makes sense, though unfortunate. figured I'd point it out
If you add both of those things to your
package.json
, they are conflicting with each other so I honestly have no idea what will happen.
well, they are not conflicting as I understand it. "heroku-run-build-script": true
tells Heroku to use the new, post-March-11th behavior, so that I know my repo won't have any surprises that day, and "heroku-postbuild": "echo Skip build on Heroku"
turns off said new behavior both now, and after March 11th (because my project isn't using your new branch from this PR yet)
well, they are not conflicting as I understand it.
@ekilah, One says "run the build script" and the other says "skip the build script". They are conflicting, and you will have to remove the "skip the build script" one in order to deploy successfully on/after March 11.
The Get Ready section in the PR description above is the best way to make this transition smoothly on March 11th.
If you don't want to follow Get Ready and deploy using this branch, then you don't need to do anything. On March 11, simply wait for the new platform behavior to be launched and this PR merged & released for this buildpack.
from https://help.heroku.com/P5IMU3MP/heroku-node-js-build-script-change-faq
Do I have to wait until March 11 to make this change? No, you can opt-in to the change anytime before then by setting the
heroku-run-build-script
key in yourpackage.json
. Opting in now will prevent any disruption to your workflow when this change is rolled out. You may remove theheroku-run-build-script
key whenever you like after March 11th. A warning will be added to the build log reminding you that it is no longer necessary once the change is made.
heroku-run-build-script
doesn't really say "run the build script" as much as it says "run the build script now instead of waiting until march 11th, when you're going to do that by default", thereby allowing me to confirm my current set up will work as expected after that date. I then disable that behavior in the same way you will have to after March 11th, without this PR. if that makes sense. I am going to use your PR branch now, and remove the "heroku-postbuild": "echo Skip build on Heroku"
, but before I do that this configuration was correct.
(I'm just clarifying here for other readers, I fully understand what needs to be done at this point. thanks for the fixes!)
I then disable that behavior in the same way you will have to after March 11th, without this PR. if that makes sense.
No this does not make sense. Setting both of those properties in your package.json
is the same effect as setting neither of them, except your build will break on March 11.
Once again, please follow the Get Ready section in the PR description above, or do nothing and it will continue to work after March 11. All of this confusion is just about deploying during the transition on March 11.
I may be able to update the command line script to detect if users are using this buildpack when it runs so that it can avoid making changes. I will look into this.
Thanks @jmorrell . Perhaps I misunderstand what you're suggesting, but I don't think we need any auto detection for this buildpack. Also, seeing your demo this morning clarified what seems to be the confusion here:
npx @heroku/update-node-build-script
should not be applied to apps deployed with master or current release of this buildpack.
This buildpack will keep working without manual changes to your app.
@mars What about an env var that you can set in this buildpack that will suppress the error message until it's removed? export SUPPRESS_NODE_RUN_BUILD_WARNING=true
@jmorrell, that would be beneficial (avoid confusion) for folks using master or registry release of this buildpack, but if they're set to an older version or their own fork then they must make the updates to avoid double builds.
@mars If I understand my own code correctly, I believe that you can add export NEW_BUILD_SCRIPT_BEHAVIOR=true
to this PR and opt everyone using this buildpack into the new behavior early.
https://github.com/heroku/heroku-buildpack-nodejs/blob/master/bin/compile#L104
Though it will also show them the "successfully opted in" message 🤔
@jmorrell sounds great. I will verify that your NEW_BUILD_SCRIPT_BEHAVIOR
code works that way 😇 and then merge this ahead of the deadline so everyone can just chill ⛄️
@jmorrell I just updated this branch with the NEW_BUILD_SCRIPT_BEHAVIOR
flag set, and sure enough this buildpack still works correctly ✅
That message that you mentioned is logged:
remote: -----> Opting in to new default build script behavior
remote: You have set "heroku-run-build-script"=true in your package.json
remote: Your app will be unaffected by the change on March 11, 2019
The second line is a bit misleading in this case, because the user didn't necessarily change their package.json
.
I'm reluctant to push this out ahead of March 11, just because so many folks are not anticipating this change until then, but this will make it so I can release this early on March 11 and have no gap in deployability.
Now updating this PR description with this new plan.
Resolves #155
Adapts this buildpack for the upcoming change to the official Node.js buildpack.
⛵️ No changes are required to apps deployed with the master version of this buildpack. The
npx @heroku/update-node-build-script
changes should NOT be applied to apps deployed with master version of this buildpack.Includes a few inherent changes to the architecture:
npm run build
automaticallydevDependencies
for auto build (and afterwards it prunes them), so a confusingNODE_ENV
workaround has been removedRelease Schedule
This feature will be merged & released on March 11, 2019, before the Node buildpack switches over, so that the transition to auto build is seamless.
Get Ready
If you want to test this, follow these instructions to switch to the new behavior ahead of its release. Otherwise, you do not need to do anything to continue using this buildpack before, on, or after March 11.
Set your app to use this buildpack branch:
⚠️ If you already set
"heroku-postbuild": "echo Skip build on Heroku"
to avoid the new auto build behavior, then make sure to remove that property frompackage.json
so that auto build can run.Finally, remember to switch the app back to the master buildpack release so that your app automatically receive buildpack updates:
Test It & Holler
Use the Get Ready instructions above to try it ahead of time and let me know if you find any problems!
Especially thank you @jmorrell for continuing to evolve the Node.js buildpack 🥓💯✅💜 and messaging so clearly about this upcoming change!