kit icon indicating copy to clipboard operation
kit copied to clipboard

feat: support for scheduled functions

Open Samuel-Martineau opened this issue 2 years ago • 6 comments

closes https://github.com/sveltejs/kit/issues/10477

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • [ ] This message body should clearly illustrate what problems it solves.
  • [ ] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Samuel-Martineau avatar Aug 02 '23 20:08 Samuel-Martineau

🦋 Changeset detected

Latest commit: fdd066346f92e28b51fb0b2fdfa03c3499d2d71d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-netlify Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Aug 02 '23 20:08 changeset-bot[bot]

Please open an issue first for new features so we can discuss the problem we are trying to solve and how we want to solve it.

A few notes on this particular PR, separate from that:

  • We use camelCase for externally visible APIs, not snake_case.
  • Conversely, we use snake_case for internal APIs and variable names, not camelCase.
  • @netlify/functions should not be a required peer dependency of this adapter.
  • You checked the box indicating that you included a changeset, but there is no changeset.

Conduitry avatar Aug 02 '23 20:08 Conduitry

@Conduitry Sorry for the mistakes, this is my first time contributing to a major open source project.

  • I am working on opening an issue about the problem, but the rough idea is to add support for Netlify scheduled functions in @sveltejs/adapter-netlify.

  • I believe I have fixed the snake_case and camelCase issues, but I may have missed some.

  • @netlify/functions should not be a required peer dependency of this adapter.

    I do not think this is possible. Netlify seems to only detect a function as scheduled if it is wrapped in the schedule function imported directly from @netlify/functions. Based on my experiments, re-exporting it or bundling it seems to prevent correct identification from Netlify.

  • I accidentaly didn't initially push the commit containing the changeset. This should now be fixed.

Samuel-Martineau avatar Aug 02 '23 21:08 Samuel-Martineau

This would need to update the README.md for adapter-netlify. It's hard for me to understand this PR without docs or a corresponding issue describing how it would be used

Also, you will need to run pnpm format to fix the lint failure

benmccann avatar Aug 09 '23 22:08 benmccann

@benmccann Sorry for the delayed response, I was away from my messages for personal reasons. I still think that it would be very interesting for Svelte Kit to have built-in support for scheduled functions on compatible platforms, but I don't have a lot of experience with open-source software and I am unsure what the next steps should be. This would require a new convention and modifications to most adapters, this PR was just an experiment of a Netlify prototype.


My suggestion would be to have something like a special src/+cron folder that would contain folders whose name are cron rules. Each folder would contain a +server.js with a POST endpoint to be executed at the schedule time.

Samuel-Martineau avatar Oct 09 '23 21:10 Samuel-Martineau

Ah, I see the issue you created now: https://github.com/sveltejs/kit/issues/10477. Thanks for the details

benmccann avatar Oct 09 '23 23:10 benmccann