feat: upgrade to sveltekit 329 #161
Conversation
Nushio
changed the title
|
I'm working on the end to end / integration tests. |
…te-adapter-firebase into prerelease/0.14.0
|
I've fixed all issues but the end-to-end tests. For some reason, the github action is using node 12 but we need 14 or 16.
Other than the node version though, I'm confident that this works on the latest sveltekit (330 as of right now) |
|
@Nushio I've seen comments that might help firebase/firebase-tools#2791 (comment) and firebase/firebase-tools#2791 (comment) |
That helps! I suspected that the issue was asdf (I've never used this before). I'll disable asdf in the meantime just to double check that e2e tests pass (they do pass in my laptop which is why I'm confident) |
|
All tests pass! Ready to go. |
|
|
||
| // Request | ||
| test('firebase-functions.https.request GET is converted to SvelteKit Incoming request type correctly', t => { | ||
| const firebaseRequest = { | ||
| method: 'GET', | ||
| headers: { | ||
| 'accept-language': 'en', | ||
| 'set-cookie': ['some', 'cookie', 'data'], | ||
| host: 'us-central1-func.cloudfunctions.net', | ||
| 'x-forwarded-proto': 'https', | ||
| }, | ||
| url: '/url?some=thing', | ||
| }; | ||
|
|
||
| const expectedKitRequest = { | ||
| method: 'GET', | ||
| headers: { | ||
| 'accept-language': 'en', | ||
| 'set-cookie': 'some,cookie,data', | ||
| host: 'us-central1-func.cloudfunctions.net', | ||
| 'x-forwarded-proto': 'https', | ||
| }, | ||
| rawBody: null, | ||
| host: 'https://us-central1-func.cloudfunctions.net', | ||
| path: '/url', | ||
| query: new URL('/url?some=thing', 'https://us-central1-func.cloudfunctions.net').searchParams, | ||
| }; | ||
|
|
||
| const result = toSvelteKitRequest(firebaseRequest); | ||
|
|
||
| t.deepEqual(result, expectedKitRequest); | ||
| }); | ||
|
|
||
| test('firebase-functions.https.request POST is converted to SvelteKit Incoming request type correctly', t => { | ||
| const firebaseRequest = { | ||
| method: 'POST', | ||
| headers: { | ||
| 'accept-language': 'en', | ||
| 'set-cookie': ['some', 'cookie', 'data'], | ||
| host: 'us-central1-func.cloudfunctions.net', | ||
| 'x-forwarded-proto': 'https', | ||
| }, | ||
| rawBody: Buffer.from('some-data', 'utf8'), | ||
| url: '/url?some=thing', | ||
| }; | ||
|
|
||
| const expectedKitRequest = { | ||
| method: 'POST', | ||
| headers: { | ||
| 'accept-language': 'en', | ||
| 'set-cookie': 'some,cookie,data', | ||
| host: 'us-central1-func.cloudfunctions.net', | ||
| 'x-forwarded-proto': 'https', | ||
| }, | ||
| rawBody: Buffer.from('some-data', 'utf-8'), | ||
| host: 'https://us-central1-func.cloudfunctions.net', | ||
| path: '/url', | ||
| query: new URL('/url?some=thing', 'https://us-central1-func.cloudfunctions.net').searchParams, | ||
| }; | ||
|
|
||
| const result = toSvelteKitRequest(firebaseRequest); | ||
|
|
||
| t.deepEqual(result, expectedKitRequest); | ||
| }); |
There was a problem hiding this comment.
I removed them because I couldn't come up with a suitable replacement. The new sveltekit library uses an Request object (different from the json provided) and I couldn't find a library to generate.
I removed them because I considered the end to end tests a bit more valuable in terms of feedback but I do regret removing them nonetheless.
There was a problem hiding this comment.
Hmm, okay, I will have to check, because the Firebase API uses ExpressJS Request object which is different to the W3C, so we might still want to have a conversion function to capture some edge cases. I wanted to re-jig the integration tests too as they were a hack just to get something working for e2e tests while Kit API was changing rapidly.
Not sure if you noticed, but I am the original person to raise that issue in Firebase repo and also maintainer of |
|
@jthegedus assume you are asking @Nushio about the last changes? Here are the two releases compared release-v0.12.1...main Perhaps a few separate issues to guide discussion would help? Not sure how structured you all want to get with the change windows? I'm still new on a lot of this, but happy to help if I stay with GCP/Firebase for my SvelteKit stuff. |
|
Yeah, was asking @Nushio . There's only ~17hrs between this PR being raised and it being merged. I realise it is merging two previous PRs from other contributors, which have been around a while and ideally I would have read before now. I have the time now to get back to my open source projects, of which this is a core project. However, I cannot commit to a review time window of <48hrs, let alone <17hrs. I am based in Australia so often find my opportunity to review code is delayed from those who raise it, which gets interpreted as unresponsive. I thought I had repo permissions set requiring additional review approvals, therefore keeping me in the loop, but it seems these were not on. Apologies if this has come off as ungrateful, I appreciate the work you have contributed @Nushio, I just need to do a lot of reading to catch up with this large PR and see if some undocumented capabilities still exist after this level of a refactor (for instance, this adapter did work with both Cloud Functions v1 & v2, but I couldn't publicly write tests for Funcs v2 as I was bound by NDA not to discuss publicly) |
Yes, I was planning on solidifying the approach/goals of this project after a I started this project back when Kit was version ~ Additionally, being in the alpha program of Firebase has meant I could see a lot of changes coming down the pipeline which will affect how this works, and I want to experiment with those myself for personal interest, more so than to provide 100% functionality of all feature requests. I am unlikely to support all feature requests of users. If I do not cover a feature I will write why in the README. 1 example of this is the function per route. That doesn't make sense to do on the Cloud Functions platform. So I probably won't bother with the overhead. I can imagine this will be the case with other features. Firebase recently added support to the You can see how this is a melting pot of different issues:
Large contributions without this knowledge can lead to divergent goals. |
Yeah, I saw your name on the asdf firebase plugin. I didn't raise an issue because I thought that the issue was with the firebase binary, not the asdf plugin. In any case, my short term solution was to use the firebase-tools package only for the end-to-end tests. This can be reverted without any issues. Particularly since Firebase tools will upgrade their binary from node 12 to node 14 in the next release iirc.
I wholeheartily apologize for this. I had free time available after work (and during work) on Friday and wanted to release the new version. Since the other PR had been merged, and my work on friday consisted on mostly stabilizing the integration / end to end tests, and merging co3k's contribution (which was mostly already been merged by the contribution from nielsvandermolen but I wanted co3k's name on the commit log / contribution list as I feel it's important. You hadn't been very active in the last few months, I didn't think it would be an issue, but it won't happen again. Feel free to remove my commiter privileges if you haven't already. I'll just send PRs if there's anything to change. I'm not offended by this, I'll continue contributing as we use this library at work, and a friend uses this library on his project as well. I love learning new technologies. I learned a lot about how the sveltekit adapters work reading the github issues and the code from sveltekit adapter for vercel, next and cloudflare. I watched Google IO specifically for flutter and firebase updates, and was happy to see the new firebase deploy feature. I'd love to help bring that feature to sveltekit if you need any help. Like I mentioned, I'm personally also interested in bringing the 'multiple codebase' feature for firebase functions into this project. I think that's an exciting change and would love if this adapter supported it in the future. I think having more contributors is important, it's why I merged co3k's PR instead of just closing it. Once again, I'm sorry if I caused any 'damage', emotional or otherwise, to your project. My intention was (and still is) to help push the adapter forward. I appreciate being given the opportunity to help with the project and would like to continue collaborating in the future. I made sure to not release a new version until A) it passed all integration tests (of which, I removed two, so I admit I cheated in that area) |
|
@Nushio No need to apologise, I have been very inactive. I appreciate your assistance and I wish to continue to have you on as a contributor as I would like to involve more people for more eyes and throughput. Having said that, until a My availability for discussions is higher than code changes as they are easier for me to do on my phone every other day, whereas code contributions require more time and a PC which I am not always near. Unfortunately I am not really going to have time to experiment until next week, if you wish to do your own experimentation in the meantime, I would appreciate your thoughts. I will start a new issue to discuss my initial thoughts. |
Summary
Update Adapter to support the latest version (next@329) of SvelteKit, thanks to nielsvandermolen and co3k for their contributions!
Updates documentation.
Bumps package dependencies.