Skip to content

feat: add option to customise URL parsing in IPX server #259

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

sgarner
Copy link

@sgarner sgarner commented Apr 4, 2025

This PR enables customisation of IPX image URLs.

When creating an H3 handler, parseUrl option can be set to a function which overrides the default URL parsing logic.

When not set, the default logic will be used, which accepts URLs in the form /<modifiers>/<id>.

As an example, below is a parseUrl function for an alternative URL style in the form /<id>@@<modifiers>.<format> — this puts the modifiers in the filename after an @@ sequence, and the format as the file extension. Such an URL style could be preferable when using IPX to prerender images for static hosting.

import { createIPXH3App, parseModifiersString } from "ipx";
import { decode } from "ufo";

createIPXH3App(ipx, {
  parseUrl: event => {
    const id = decode(event.path.slice(1 /* remove leading slash */));

    const matches = id.match(/^(.+)@@(.+)\.([^.]+)$/);
    if (matches == null) {
      return {
        id,
        modifiers: {},
      };
    }
    
    const modifiers = parseModifiersString(matches[2]);
    modifiers.format = matches[3];
    delete modifiers.f;

    return {
      id: matches[1],
      modifiers,
    };
  }
});

Resolves #54, #160

Copy link

codecov bot commented Apr 4, 2025

Codecov Report

Attention: Patch coverage is 65.85366% with 14 lines in your changes missing coverage. Please review.

Project coverage is 48.55%. Comparing base (f666cb4) to head (6150ef7).
Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
src/server.ts 65.85% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #259      +/-   ##
==========================================
- Coverage   56.02%   48.55%   -7.48%     
==========================================
  Files          14       13       -1     
  Lines        1203      898     -305     
  Branches       83       91       +8     
==========================================
- Hits          674      436     -238     
+ Misses        526      459      -67     
  Partials        3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pi0
Copy link
Member

pi0 commented Apr 4, 2025

This is a nice idea! We probably need to introduce it for the next major version, as new behavior can still potentially conflict with valid file names.

Also what do you think to use --[modifiers].ext as less verbose alternative test.png--s_300x300.webp looks more readable to me than test.png@@s_300x300.webp

@pi0
Copy link
Member

pi0 commented Apr 4, 2025

I will also think more about whole implementation part, we might make URL parser configurable to avoid need of ~ only to support both styles at the same time.

@pi0 pi0 marked this pull request as draft April 4, 2025 08:32
@sgarner
Copy link
Author

sgarner commented Apr 4, 2025

I will also think more about whole implementation part, we might make URL parser configurable to avoid need of ~ only to support both styles at the same time.

I agree, making it configurable would be nice. 🤔 We could provide parsers for the old and new style, and allow the user to create their own.

How would you feel about this schema?

export type IPXOptions = {
  urlPathParser?: 'modifiers-in-path' | 'modifiers-in-filename'
    | ((path: string) => { modifiers: Record<string, string>; id: string; })

  // ...
}

Also what do you think to use --[modifiers].ext as less verbose alternative test.png--s_300x300.webp looks more readable to me than test.png@@s_300x300.webp

There is an established convention of naming image files like [email protected], e.g. [email protected] or [email protected] (I believe Apple started this, and tools like Figma support it). So I proposed using @@modifiers as a nod to this, but I don't mind either way.

@sgarner
Copy link
Author

sgarner commented Apr 4, 2025

Regarding the change in SVG logic (comment has disappeared?)...

One caveat with the new URL style is that it requires the target format to be specified. It wouldn't be right to request /_ipx/~/images/test.png@@w_300 for example.

The previous logic allowed SVG files to be returned as-is (or optimised with SVGO) when the target format was omitted:

/_ipx/w_300/test.svg
-> sends back test.svg as unmodified/optimised SVG file

But it didn't allow svg to be used as a target format (it's not listed in SUPPORTED_FORMATS). Attempting to do so would default to JPEG instead:

/_ipx/w_300&f_svg/test.svg
-> returns a JPEG version

I revised the logic so that svg can be used as a target format when the source format is also svg. But still not allowing it as a target for other sources.

Which permits the following to work:

/_ipx/~/test.svg@@w_300.svg
-> sends back test.svg as unmodified/optimised SVG file

@pi0
Copy link
Member

pi0 commented Apr 6, 2025

Thanks for explanations.

Can you update PR to only add support for urlPathParser (i would call it simply parseURL)? (no change in svg part we can do in another PR and also not named presets, only if not set, we have default parser for current behavior)

@sgarner sgarner changed the title feat: add alternate URL style with format as file extension feat: add option to customise URL parsing in IPX server Apr 8, 2025
@sgarner
Copy link
Author

sgarner commented Apr 8, 2025

@pi0 I have revised this PR to just add the parseUrl option, so users can define their own URL style.

@sgarner sgarner marked this pull request as ready for review May 2, 2025 21:18
@sgarner
Copy link
Author

sgarner commented May 2, 2025

@pi0 I just realised this PR was still in draft mode. It is ready for review when you can

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 this pull request may close these issues.

Change file extension when converting format
2 participants