fix: Serverless Deployment provider packs correctly #10646
Conversation
|
Thanks @jwwisgerhof for fixing! I don't have a serverless deployed app to test this. Could you provide the logs are some evidence of this fixing working (redacted for any sensitive info) so we can review approve and merge. |
|
Hard to provide logs that show a definite fix, but have tested this on our enterprise app and am using it live at the moment. WIthout fix: With my branch built and lifted into our project: I'll set up a test repo with minimal code and link it here. |
|
Alright @dthyresson - gutted our repo quickly, probably a bit messy but it serves the purpose. To reproduce in either repo:
Succeeds on main, fails on RW-75 branch. Build my PR and copy dist over to RW-75 branch and deploy command succeeds. Let me know if you need more info :) EDIT: Tests failing as it was watching the export. Will look into this later when I get a chance |
|
Thank you for working on this @jwwisgerhof! |
| @@ -7,6 +7,8 @@ import fse from 'fs-extra' | |||
| import { findApiDistFunctions } from '@redwoodjs/internal/dist/files' | |||
| import { ensurePosixPath, getPaths } from '@redwoodjs/project-config' | |||
|
|
|||
| import * as nftPacker from '../packing/nft' | |||
There was a problem hiding this comment.
Do we have any idea about what changed and broke this in the first place? This is a surprising fix given that the error showed up after a minor release... Assuming the culprit could be with the NFT package itself. 🤷♂️
No need to hold up the PR to try and figure it out — goal here is simple + efficient. Just thought it was worth asking.
There was a problem hiding this comment.
I am not 100% sure - I got to this fix by simple trial and error, realizing eventually that "exports" is undefined. Having a look around the web now , it has to do with how the JS is compiled / emitted. Modern versions don't allow the exports.xxx syntax depending on module resolution.
I am not familiar enough with the RW codebase to see what change might have caused this. I did go through our build log and can confirm that in @redwoodjs/cli@npm:7.2.0-rc.53 this still worked and packed correctly.
There was a problem hiding this comment.
it has to do with how the JS is compiled / emitted
Yeah, this is the kind of thing that has me thinking a bit; one of our related build dependencies could have been updated behind the scenes and introduced a behavior change that's breaking.
Again, not a critical priority to figure out. Appreciate you taking time to reply 👍
dthyresson
changed the title
dthyresson
changed the title
|
@jwwisgerhof One last thing before I can merge. Might you add a changeset so the details of this fix gets into the release notes? See: https://github.com/redwoodjs/redwood/blob/main/.changesets/10668.md for examples. Appreciate it! |
…jwwisgerhof/redwood-serverless-fix into jwwisgerhof-serverless-pack-fix
|
Thanks @dthyresson - have added that now. Looking forward to getting this released and upgrading to RW7! |
Fixes #10528 Simple fix that restores Serverless deployment functionality. --------- Co-authored-by: David Thyresson <dthyresson@gmail.com>
Fixes #10528 Simple fix that restores Serverless deployment functionality. --------- Co-authored-by: David Thyresson <dthyresson@gmail.com>
Fixes #10528
Simple fix that restores Serverless deployment functionality.