fix: Replace slugify in deploy-preview-netlify with simple regex solution #1543

saibotk · 2022-02-14T13:59:36Z

saibotk commented

2022-02-14 13:59:36 +00:00

This commit removes the direct dependency on the "slugify" package, since it produces slugs, which are still invalid to be used as subdomain names.

We now use a custom RegEx scheme which just replaces all white spaces, dots and slashes with a "-" sign for readability and removes all otherwise invalid characters.

To account for specific language characters or unicode variants we first normalize it using the String.prototype.normalize() function, so we can preserve as much characters as possible.

Additionally, we now make sure that our alias is not longer than 37 characters, which is the maximum alias length according to Netlify's documentation 0

Fixes #1058

Example/comparison:

This commit removes the direct dependency on the "slugify" package, since it produces slugs, which are still invalid to be used as subdomain names. We now use a custom RegEx scheme which just replaces all white spaces, dots and slashes with a "-" sign for readability and removes all otherwise invalid characters. To account for specific language characters or unicode variants we first normalize it using the String.prototype.normalize() function, so we can preserve as much characters as possible. Additionally, we now make sure that our alias is not longer than 37 characters, which is the maximum alias length according to Netlify's documentation [0] [0]: https://cli.netlify.com/commands/deploy Fixes #1058 Example/comparison: ![image](/attachments/7edef352-3c65-4d19-9c7a-be3d6f9da7fa)

image.png

34 KiB

saibotk changed title from ~~Replace slugify with simple regex solution~~ to deploy-preview-netlify: Replace slugify with simple regex solution

2022-02-14 14:04:48 +00:00

saibotk changed title from ~~deploy-preview-netlify: Replace slugify with simple regex solution~~ to fix: Replace slugify in deploy-preview-netlify with simple regex solution

2022-02-14 14:06:58 +00:00

saibotk force-pushed fix/netlify-preview-slugs from ce8500a640 to a33e1e45bc

2022-02-14 14:11:12 +00:00

Compare

frederick commented

2022-02-14 14:15:16 +00:00

Hi saibotk!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1543-fix-netlify-preview-slugs--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi saibotk! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1543-fix-netlify-preview-slugs--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.

dpschen commented

2022-02-14 14:30:35 +00:00

Nice, thanks! =)

From my side this is good as long as it works better than now (which is basically not :D).

One little thing:
@saibotk Maybe you can add the relevant comments from above directly in the file for future reference.

Nice, thanks! =) From my side this is good as long as it works better than now (which is basically not :D). One little thing: @saibotk Maybe you can add the relevant comments from above directly in the file for future reference.

👍 1

dpschen requested review from konrad 2022-02-14 14:30:46 +00:00

konrad was assigned by dpschen

2022-02-14 14:30:49 +00:00

konrad approved these changes 2022-02-14 18:08:47 +00:00

konrad left a comment

Replacing dependencies is always great! Thanks!

konrad merged commit 28af46bcd3 into main

2022-02-14 18:09:21 +00:00

konrad deleted branch fix/netlify-preview-slugs

2022-02-14 18:09:21 +00:00

konrad referenced this issue from a commit

2022-02-14 18:09:21 +00:00

fix: Replace slugify in deploy-preview-netlify with simple regex solution (#1543)

This repo is archived. You cannot comment on pull requests.

4 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#1543