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

(adapter-netlify): _redirects file gets overwritten instead of changed #1585

Closed
roschaefer opened this issue May 29, 2021 · 3 comments · Fixed by #1586
Closed

(adapter-netlify): _redirects file gets overwritten instead of changed #1585

roschaefer opened this issue May 29, 2021 · 3 comments · Fixed by #1586

Comments

@roschaefer
Copy link
Contributor

roschaefer commented May 29, 2021

Describe the bug
The documentation clearly says:

During compilation a required "catch all" redirect rule is automatically appended to your _redirects file. (If it doesn't exist yet, it will be created.)

See: https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#using-netlify-redirect-rules

This is not quite true. When I put my own _redirects into either the root directory or build/ folder, the file gets overwritten.

To Reproduce

  1. Create a file build/_redirects and add some content:
/workshops    kategorie=:kategorie /.netlify/functions/render 200!
  1. Run svelte-kit build
  2. Content of build/ is:

/* /.netlify/functions/render 200

If possible, we recommend creating a small repo that illustrates the problem.

You can clone my repository and checkout my commit: roschaefer/tanzimpulse@984ba91

Expected behavior

Expected content of build/_redirects should be:

/workshops    kategorie=:kategorie /.netlify/functions/render 200!

/* /.netlify/functions/render 200

Information about your SvelteKit Installation:

Diagnostics
  • The output of npx envinfo --system --npmPackages svelte,@sveltejs/kit,vite --binaries --browsers
$ npx envinfo --system --npmPackages svelte,@sveltejs/kit,vite --binaries --browsers
Need to install the following packages:
  envinfo
Ok to proceed? (y) y

  System:
    OS: Linux 5.12 Arch Linux
    CPU: (12) x64 AMD Ryzen 5 PRO 4650U with Radeon Graphics
    Memory: 18.48 GB / 30.65 GB
    Container: Yes
    Shell: 3.2.2 - /usr/bin/fish
  Binaries:
    Node: 14.15.4 - ~/.asdf/installs/nodejs/lts/bin/node
    Yarn: 1.22.10 - ~/.asdf/installs/nodejs/lts/.npm/bin/yarn
    npm: 6.14.10 - ~/.asdf/installs/nodejs/lts/bin/npm
  Browsers:
    Chromium: 91.0.4472.77
    Firefox: 88.0.1
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.109
    svelte: ^3.38.2 => 3.38.2
  • Your browser
    Irrelevant

  • Your adapter (e.g. Node, static, Vercel, Begin, etc...)
    It's adapter-netlify

Severity

A bit unfortunate but not a blocking issue. For my use case, this would allow to prerender sites (+performance) while catering noscript-browsers(+accessiblity).

Additional context
I asked at Discord: https://discord.com/channels/457912077277855764/819723698415599626/848204698808352788

Hey everyone 👋 does anybody know if it's possible to configure netlify-adapter in a way that /workhops gets served by a pre-rendered page but /workshops?filter=something hits the serverless function?

I figured it out myself! In build/_redirects:

/workshops    kategorie=:kategorie /.netlify/functions/render 200!

/* /.netlify/functions/render 200

The relevant documentation is here: https://docs.netlify.com/routing/redirects/rewrites-proxies/#shadowing

Suggestions to Fix

I'm quite sure it's because the entire build/ folder gets deleted here:

utils.rimraf(publish);

appendFileSync would never find an existing file:
appendFileSync(`${publish}/_redirects`, '\n\n/* /.netlify/functions/render 200');

Furthermore, I would suggest that users can put a _redirects file into the project root folder instead of the build/ folder. This would allow users to .gitignore the build/ folder.

roschaefer added a commit to roschaefer/kit that referenced this issue May 29, 2021
This was easier to fix than expected. The fix should allow users to
have their own `_redirects` file in the project root. If this file
exists, it gets copied to `build/` before this adapter appends the
catchall rule to it.

This behaviour is [already documented](https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#using-netlify-redirect-rules), but [currently broken](sveltejs#1585).

How can I write an automated test for this adapter?

I did the following to test this manually:

1. Use `yarn link "@sveltejs/kit"` and `yarn link
   "@sveltejs/adapter-netlify"` in my application repository to point
   the two node modules to my modified versions.
2. Run `svelte-kit build` with a `_redirects` file in the project root.
3. Check if content of `build/_redirects` is expected.
4. Run `svelte-kit build` *without* a `_redirects` file in the project root.
5. Check if content of `build/_redirects` is expected.

Any guidance or help how to match the desired code quality to get this
PR merged is appreciated.

close sveltejs#1585
roschaefer added a commit to roschaefer/kit that referenced this issue May 29, 2021
This was easier to fix than expected. The fix should allow users to
have their own `_redirects` file in the project root. If this file
exists, it gets copied to `build/` before `adapter-netlify` appends
the catchall rule to it.

This behaviour is [already documented](https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#using-netlify-redirect-rules), but [currently broken](sveltejs#1585).

How can I write an automated test for this adapter?

I did the following to test this manually:

1. Use `yarn link "@sveltejs/kit"` and `yarn link
   "@sveltejs/adapter-netlify"` in my application repository to point
   the two node modules to my modified versions.
2. Run `svelte-kit build` with a `_redirects` file in the project root.
3. Check if content of `build/_redirects` is expected.
4. Run `svelte-kit build` *without* a `_redirects` file in the project root.
5. Check if content of `build/_redirects` is expected.

Any guidance or help how to match the desired code quality to get this
PR merged is appreciated.

close sveltejs#1585
roschaefer added a commit to roschaefer/kit that referenced this issue May 29, 2021
This was easier to fix than expected. The fix should allow users to have their own `_redirects` file in the project root. If this file exists, it gets copied to `build/` before `adapter-netlify` appends the catchall rule to it.

This behaviour is [already documented](https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#using-netlify-redirect-rules), but [currently broken](sveltejs#1585).

How can I write an automated test for this adapter?

I did the following to test this manually:

1. Use `yarn link "@sveltejs/kit"` and `yarn link "@sveltejs/adapter-netlify"` in my application repository to point the two node modules to my modified versions.
2. Run `svelte-kit build` with a `_redirects` file in the project root.
3. Check if content of `build/_redirects` is expected.
4. Run `svelte-kit build` *without* a `_redirects` file in the project root.
5. Check if content of `build/_redirects` is expected.

Any guidance or help how to match the desired code quality to get this PR merged is appreciated.

close sveltejs#1585
roschaefer added a commit to roschaefer/kit that referenced this issue May 29, 2021
This was easier to fix than expected. The fix should allow users to have their own `_redirects` file in the project root. If this file exists, it gets copied to `build/` before `adapter-netlify` appends the catchall rule to it.

This behaviour is [already documented](https://github.com/sveltejs/kit/tree/master/packages/adapter-netlify#using-netlify-redirect-rules), but [currently broken](sveltejs#1585).

How can I write an automated test for this adapter?

I did the following to test this manually:

1. Use `yarn link "@sveltejs/kit"` and `yarn link "@sveltejs/adapter-netlify"` in my application repository to point the two node modules to my modified versions.
2. Run `svelte-kit build` with a `_redirects` file in the project root.
3. Check if content of `build/_redirects` is expected.
4. Run `svelte-kit build` *without* a `_redirects` file in the project root.
5. Check if content of `build/_redirects` is expected.

Any guidance or help how to match the desired code quality to get this PR merged is appreciated.

close sveltejs#1585
@kvn-shn
Copy link
Contributor

kvn-shn commented May 29, 2021

I believe there is a small misunderstanding. :) The _redirects file belongs into the project's static folder (not build nor your project's root).

@roschaefer
Copy link
Contributor Author

@kvn-shn you're right! Should the documentation be updated to avoid further confusion? Or is it just me and we can assume that people understand that they have to put netlify configuration files into /static?

@roschaefer
Copy link
Contributor Author

I'm going to close this non-issue. Feel free to ask me if I should send a PR to update docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants