[fix] revert adapters automatically updating .gitignore (#1924)

[Rich-Harris]

Apologies for missing this when #1924 was still open. It violates something I've learned the hard way to be a sacrosanct rule: a given file in a project should be edited by humans, or by machines, but never by both after the project has been created (package.json might seem like an obvious counter-example, but appending dependencies is something the package manager does as a direct consequence of your command; it doesn't get unexpected edits anywhere else). We really can't go around editing people's code, even in trivial-seeming ways in ignore files.

Adding ignored directories seems simple and uncontroversial enough to be a no-brainer. But even in #1924 itself it introduced unwanted changes:

/.svelte-kit
/build
/functions

+build

Here, the /build line already excluded the build directory from version control. The inserted line is duplicative. Worse than that, it will cover any folder called build anywhere in your project, so if your app had a route whose name coincided with that fairly common English word, it would now be excluded as well. As someone who has lost work because of incorrectly-excluded folders in the past, I would be pretty irate if it happened again because my framework took it upon itself to 'fix' my ignore file. We could robustify it, but there'll always be some edge case.

Beyond that, someone might actually want the build artifacts to be checked in. In fact I work on a team where that's the norm! (It's a practice I've railed against, but there are reasons that it's done that way, and the fact that it's a thing that people do, however unconventional, outweighs any arguments about why they shouldn't.) While the documentation tells you that you can comment the line out, most people would simply delete the line, only for it to be reinserted. Fighting your tools is a bad feeling.

Here's what I think we should do instead:

• Where it makes sense, adapters should place their output inside .svelte-kit (i.e. .svelte-kit/netlify, etc) so that they are already ignored by ignore files
• Check ignore files to see if we're writing to locations that aren't already excluded, and print a loud warning if they're not. We're using gitignore-parser in create-svelte to do this robustly:

  kit/packages/create-svelte/scripts/build-templates.js

  Lines 49 to 50 in 3e30a1b

  	// ignore contents of .gitignore or .ignore
  	if (!gitignore.accepts(name) || !ignore.accepts(name) || name === '.ignore') return;

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
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 23b296f

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

This PR includes changesets to release 6 packages

Name	Type
@sveltejs/adapter-cloudflare-workers	Patch
@sveltejs/adapter-netlify	Patch
@sveltejs/adapter-node	Patch
@sveltejs/adapter-static	Patch
@sveltejs/adapter-vercel	Patch
@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

[benmccann]

I'm not sure you should revert the changeset from the original PR and this one will need it's own changeset. Other than that, LGTM

Where it makes sense, adapters should place their output inside .svelte-kit (i.e. .svelte-kit/netlify, etc) so that they are already ignored by ignore files

So obvious in retrospect. I'm embarrassed none of us thought of it in. I started us off by updating the Netlify adapter here: #2058

[benmccann]

FYI @JeanJPNM, @jthegedus

[pngwn]

Like @benmccann we should fix forward (even though this is a revert) and add a new changeset for this.

[JeanJPNM]

Seems reasonable, but I see this more as a bug, since the code duplicates paths that are relative to the root. But implementing that could make the process a lot slower. And manually stating that the path is relative to the root of the project on the adapter configuration feels like a waste of time. Maybe reverting the changes its the best option.

[benmccann]

Like @benmccann we should fix forward (even though this is a revert) and add a new changeset for this.

While forward fixing is nice, I think it's more important to get the change out before folks have their .gitignore files updated. We can go back and implement the new way afterwards

I pushed some changes and may merge this pretty soon to get it out

[jthegedus]

Should we expect to see gitignore-parser provided via the Adapter API in the near future?

[Rich-Harris]

Yes, I think we should — we could have an API like utils.warn_if_unignored([opts.output])

[jthegedus]

Sounds like a good soln. Hopefully loud CLI output is enough.