kit icon indicating copy to clipboard operation
kit copied to clipboard

fix: disallow exporting `actions` from a `+layout.server` file

Open teemingc opened this issue 2 years ago • 6 comments

~Found out we can export actions in a layout file but it isn't mentioned anywhere in the docs.~ I don't think we're suppose to export actions from a layout file. https://github.com/sveltejs/kit/pull/10046#issuecomment-1565286897

This PR changes the list of valid exports to disallow the actions export from a +layout.server file (matching the language tools behaviour).

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

  • [ ] 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
  • [x] This message body should clearly illustrate what problems it solves.
  • [x] 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:.

teemingc avatar May 27 '23 08:05 teemingc

🦋 Changeset detected

Latest commit: 58cd9c5b6e619ea217786d84976bfae5f391d3eb

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

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 May 27 '23 08:05 changeset-bot[bot]

I don´t think you can do that. Vscode gives me this error:

Invalid export 'actions' (valid exports are prerender, ssr, csr, trailingSlash, config, load, or anything with a '_' prefix

david-plugge avatar May 27 '23 08:05 david-plugge

Huh. That's weird. It's suppose to be valid based on the code here: https://github.com/sveltejs/kit/blob/f8cf1fec360961724fb00d3b47513a4058cfd411/packages/kit/src/utils/exports.js#L72 and it's tested here https://github.com/sveltejs/kit/blob/f8cf1fec360961724fb00d3b47513a4058cfd411/packages/kit/src/utils/exports.spec.js#L114-L118

teemingc avatar May 27 '23 08:05 teemingc

Not sure if we need to update language tools to allow actions in layout or update kit to disallow actions in layout. https://github.com/sveltejs/language-tools/blob/98b925617bbdbf6e9e51a1d7e229d0f27c2337cc/packages/typescript-plugin/src/language-service/sveltekit.ts#L280-L281

teemingc avatar May 27 '23 08:05 teemingc

I wonder how actions in layout files would work, specifically how you would call them from the client (what to put in the form action attribute?). Page and layout actions could collide, how would the server choose between src/routes/#layout.server.ts#actions.signin and src/routes/#page.server.ts#actions.signin ?

david-plugge avatar May 27 '23 12:05 david-plugge

I... would expect you shouldn't export actions from layout components? I have no idea what URL they would / should live at.