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

feat(qwik-city): init Fastly adapter #5552

Closed
wants to merge 6 commits into from

Conversation

kalebpace
Copy link

@kalebpace kalebpace commented Dec 9, 2023

Overview

Hi everyone,

I've initialized a Qwik City adapter for Fastly's Compute@Edge platform based off the Cloudflare adapter. All code for the adapter and e2e usage lives here and is deployed here.

There is a decent amount of work still needed to bring the adapter and starter template up to speed for a proper dev server and production deployment. Though, wanted to open this PR early in the process to get feedback and advice on how best to integrate Fastly specific adapter needs (e.g. adding fastly.d.ts to the root Qwik project since @fastly/js-compute only exposes types for virtual modules like fastly:env).

I have additional work here that is slightly older, but had the intention of opening a PR into the @fastly/js-compute project to address type issues when trying to import functions using static imports, otherwise dynamic imports are needed. Likely the next step to bringing the adapter up to speed.

Thanks in advance for your time, look forward to hearing any feedback!

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

- Added @fastly/js-compute and fastly.d.ts to root so API Extractor can be aware of types during generation
- Added @fastly/js-compute and @fastly/compute-js-static-publish to qwik-city for use with adapter middleware
- middleware uses @fastly/compute-js-static-publish to handle inlining static assets into the WASM or publishing via Fastly KV Store
Copy link

netlify bot commented Dec 9, 2023

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit f15a6c0

@kalebpace kalebpace changed the title feat(qwik-city): init Fastly adapter feat(qwik-city): init Fastly adapter Dec 9, 2023
@gioboa
Copy link
Member

gioboa commented Dec 11, 2023

Thanks @kalebpace for this integration
here you can find the e2e suite to test if the integration covers everything.

@kalebpace
Copy link
Author

kalebpace commented Dec 11, 2023

Thanks @gioboa! I'll give this a go soon.

If I get things working, should I also open a PR into that repo with a link, plus any other changes, to the environment the E2E tests are run against?

Oops, will model contributions after the Fastify work here:
- #3282
- QwikDev/qwik-city-e2e#7

Update:
QwikDev/qwik-city-e2e#9

@gioboa
Copy link
Member

gioboa commented Dec 24, 2023

@kalebpace do you have any news for this PR?

@kalebpace
Copy link
Author

@gioboa thanks for checking-in.

Unfortunately the Fastly runtime has a few APIs missing to fully cover the suite.

I've outlined what is missing here: QwikDev/qwik-city-e2e#9
And opened an issue with Fastly: fastly/js-compute-runtime#711

Waiting to get some feedback/suggestions on whether the adapter is acceptable in its current state, maybe tagged as experimental, or if I need to put this work on pause and coordinate with Fastly until APIs are implemented.

Happy to hear any thoughts you might have on this!

@gioboa
Copy link
Member

gioboa commented Dec 26, 2023

I think it's better to ship the feature only when is complete. This avoids unpleasant issues for missing features/bugs.

@wmertens wmertens added TYPE: enhancement New feature or request WAITING FOR: user Further information is requested from the issue / pr opener labels Mar 4, 2024
@gioboa
Copy link
Member

gioboa commented May 27, 2024

Hi @kalebpace thanks for your help.
Is this PR still valid or is it abandoned?

@kalebpace
Copy link
Author

It's likely invalid, good to close

@gioboa gioboa closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TYPE: enhancement New feature or request WAITING FOR: user Further information is requested from the issue / pr opener
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants