Skip to content

transformPage #3511

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

Closed

Comments

@Rich-Harris
Copy link
Member

Describe the problem

Until recently, the way to manipulate HTML was to do something like this in handle:

export async function handle({ request, resolve }) {
  const rendered = await resolve(request);

  if (rendered.headers['content-type'] === 'text/html') {
    rendered.body = rendered.body.replace(/old/g, 'new');
  }

  return rendered;
}

With #3384, we're now dealing with Response objects meaning that you'd now do this:

export async function handle({ event, resolve }) {
  let response = await resolve(event);

  if (response.headers.get('content-type') === 'text/html') {
    const body = await response.text();
    response = new Response(body.replace(/old/g, 'new'), {
      status: response.status,
      headers: response.headers
    });
  }

  return response;
}

For all the advantages brought by using Request and Response, this does not feel like an improvement. Verbosity aside, doing await response.text() and creating a new response feels like a lot of overhead.

Describe the proposed solution

We could do this:

export async function handle({ event, resolve }) {
  return await resolve(event, {
    transformPage: ({ html }) => html.replace(/old/g, 'new')
  });
}

Then this line...

const html = options.template({ head, body, assets });
...would become something like

const html = options.transformPage(options.template({ head, body, assets }));

Alternatives considered

The alternative is to do nothing. It depends on whether we consider manipulating the HTML response to be 'supported' — if it's a rarely-used hack, perhaps it's not necessary, but if we consider it to be a valid and common use case then the API probably pays for itself.

Minification is one use case; injecting the correct <html lang="..."> attribute is another (albeit one that Kit will eventually have full support for anyway). I've seen it be used for other stuff — here's a snippet from the NYT covid tracker that allows multiple Kit apps to exist on the same page...

/** @type {import('@sveltejs/kit').Handle} */
export async function handle({ request, resolve }) {
  const rendered = await resolve(request);

  if (typeof rendered.body === 'string') {
    rendered.body = rendered.body
      .replace(/start\({([\s\S]+?)}\);/, (match, $1) => {
        const opts = $1.replace(/target: .+?,/, 'target,');
        return `const target = document.querySelector('#__covidtracker__');\n\t\t\ttarget.removeAttribute('id');\n\t\t\tstart({${opts}});`;
      })
      .replace(/__covidtracker__/g, `svelte-${hash(request.path)}`);
  }

  return rendered;
}

...and there are doubtless other uses we can't anticipate. So I vote for its inclusion, but others might feel differently.

Importance

nice to have

Additional Information

No response

@ebeloded
Copy link

Thank you for pointing out how it can be done after #3384!

Yes, please. This would allow for a better way to handle templating of app.html.

Here's what it would look like to allow ejs syntax templating in app.html:

export async function handle({ event, resolve }) {
  return await resolve(event, {
    transformPage: ({ html }) => ejs.render(html, { foo: 'bar' }))
  });
}

@rodoch
Copy link

rodoch commented Jan 24, 2022

I also think this would be useful. I have to do these kinds of operations in almost every SK app I build and this syntax is concise while making the intent clearer.

@itssumitrai
Copy link

Definitely very useful for my use case as well. Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment