kit icon indicating copy to clipboard operation
kit copied to clipboard

fix: skip broken files/symlinks in list_files function

Open jwbargsten opened this issue 1 year ago • 2 comments

The list files function walks through a directory tree and finds/filters all files. If it encounters a symlink, it fails with an exception:

ENOENT: no such file or directory, stat ...

That can be cumbersome, because if one of your files in static/ is a broken symlink you are not able to test your app anymore via e.g. npm run dev.


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:.

Edits

  • [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

jwbargsten avatar Feb 18 '24 12:02 jwbargsten

🦋 Changeset detected

Latest commit: fa85c89a17f3e309a1b4302ae9cce6bd38adb678

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 Feb 18 '24 12:02 changeset-bot[bot]

I'm worried this will result in people deploying apps with missing assets to production

I think that it might be out of scope for SvelteKit to figure out if all assets are correct.

  • I compare a list_files function more to the unix ls command that also will work if some files/links are broken. Especially if it comes to files that are out of your control, e.g. hidden files placed by a Kubernetes or FUSE mount.
  • It is probably not an error at all: the symlink in itself is correct. Only the target file is not there. The function fails because it does a stat call on it to figure out if it is a dir or not. I also use git annex where it is fairly normal to have broken symlinks (because files are not present on the system, currently)

But on the other hand it might be better to move directories that might contain these issues completely out of SvelteKit projects and supply a separate server/API for it. The assets dir is indeed for assets only. Unfortunately it is called static/ and not assets/ and it might be misleading for beginners.. ;)

Which would then mean in conclusion this PR could be closed without merging....

jwbargsten avatar Feb 18 '24 15:02 jwbargsten