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
Bug prevents rendering in Safari #6039
Comments
Thanks for the detailed report @rodmachen ! Reproducible on the master branch of Added a comment here IonicaBizau/parse-url#20 (comment) saying that the upgrade was a breaking change. @mtlewis has a suggestion that we can experiment with adding resolutions in our plugin libraries (catalog, scaffolder) to prevent the version bump and hope that fixes for all the Backstage instances. But not sure if that would work! |
@OrkoHunter I wonder if this was fixed by the following PR? IonicaBizau/parse-url#28 |
I think so too. parse-url@5.0.* is using the correct version of normalize-url v4 which is not broken on Safari. https://github.com/IonicaBizau/parse-url/blob/5.0.6/package.json#L35 |
Based off of the possible solution given, I used: "resolutions": {
"**/git-up": "4.0.2"
}, in my top level package.json and that has worked around the issue for me until something better comes along, doing it at the parse-url package level gave a warning so I went with git-up. In case it's useful for other passers-by. |
This is still not working in Safari with the latest backstage component versions updated, just a heads up |
Yeah I dug around a bit again on this, and I poked at them a bit recently to see if we can get this moving remotely somehow, and also pinged the actual transitive problematic package maintainer about whether he'd be amenable to fixing the problem at its root. |
Update - i got a fix merged and released to remove the safari-breaking regex. Only to discover that they exclusively dist as ESM now which our dependency isn't set up to consume, being purely CJS and not having a build/transpile setup. So uh, that's where we are right now. Not sure how to best proceed. |
Because of @freben 's excellent work, I think right now the easiest solution could be add this to your
Then reinstall the dependencies by |
@mapleeit EDIT: This may actually be a functioning fix now that ESM support is in place. |
Bumping this since it's affecting us as well and I can't seem to get the above solution in place for the reasons @freben mentioned. Is there a possible fix here and/or cli/tool upgrades that I missed in the referenced issue/PR? |
Oh wow, that's obscure! Thanks for digging into it. Would you like to contribute that change in a pull request? Should be safe since they are functionally equivalent. |
Sure :) Been meaning to contribute for a while now, this is a good time as any to get started |
I wanted to update this issue as this is the goto based on the other closed issues. The frontend can load, but the backend cannot start.
I have worked around it by pinning |
My fix is to add
To the top level |
We ran into this issue with the most recent version:bump. @agates4 top level package.json (above) fixed for now. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Asking partially to try to prevent this from closing, but would be nice if this one stayed open at least so people can discover the workaround with the pinned resolution. |
The resolutions put forward by agates4 work for us - but When bumping the package version to either the latest (7.0.2) or to ^6.0.1, where the vulnerability was patched, the Safari bug reoccurs. Posting for visibility - the package resolution that fixed this issue for us is not secure. |
@gifteconomist could you raise this exact concern with the upstream library to track what you're seeing there? I wonder if they've just missed this? |
@benjdlambert The vulnerability has been fixed in later major versions of |
Yes, and that concern is what I mean you should bring up with the upstream library, that this is still broken in the later versions. It's not something that we control, so we should track this issue upstream to get if fixed, and then we can push out the version bump here. |
@benjdlambert Gotcha. Looks like they're working on it - IonicaBizau/parse-url#32 |
Interesting that if they're not going to backport the fix to packages without esm support we might need to wait for this: #12218 also |
Great workaround, the only problem with that, that parse-url has critical vulnerability on this version :( |
FYI: the issue IonicaBizau/parse-url#32 was closed and a new |
Sounds like this ready to go! Does anyone want to pick this up? 🙏 |
@jhaals upon further inspection it looks like we're still waiting for privatenumber/parse-url#1 to make it to upstream. And then it should be ready I believe? |
I think you pointed to a fork? I dug in a little bit and came across IonicaBizau/git-up#34 |
@Pike, yeah his fork is awaiting feedback from the original author and then he was gonna raise a PR to the upstream library. Not sure if that's a dependency on this ticket though or if your linked PR is enough 🙏 |
The regex is in a PR upstream, IonicaBizau/parse-url#59. |
It looks like the fix in the upstream was merged, and added to Could this by chance make it to |
I've raised the above PR to bump all plugins in this repo, but any other plugins will also need bumping if they depend on it directly, or depend on some of our packages with an outdated version too 🙏 |
Hi. Would it be possible to detect Safari and return a simple text document with 'Please switch to xxx'? This would avoid wasting time when this problem is re-discovered. |
Isn't it fixed in the latest release? |
It is meant to be, yeah. If it isn't, we're of course happy to have it reported separately with more additional details. |
run into this today, latest backstage, safari 16.1. hard to follow the thread, what is the latest resolution? SyntaxError: Invalid regular expression: invalid group specifier name |
@alexex I think that the fix is to bump |
thanks, I have:
and
so I'm guessing this is a new issue, I'll create one |
Can you do the same for |
We seem to have started hitting the issue as well.
|
@ljupchokotev can you check #19266 instead |
backstage#6039 (comment) Solves: Jira BS-67 Change-Id: I991011650f8e1fdf1304f91d5666ffcfbe9e6be6
A recent bump of a nested dependency caused our Backstage instance to fail in Safari. This could happen to a future deployment of the opensource Backstage app.
Expected Behavior
Safari should be supported by Backstage but this bug prevents it from being used.
Current Behavior
It appears that if
normalize-url
gets bumped to 6.0.1, an "irregular regex" error happens, preventing any of the UI from displaying in Safari. This version gets pulled in whenparse-url
gets bumped from 5.0.2 to 5.0.3.parse-url
is used bygit-up
which is used bygit-url-parse
which is used by several OS components including@backstage/plugin-catalog
and@backstage/plugin-scaffolder
.Possible Solution
We are using a yarn resolution to pin parse-url to 5.0.2 but that doesn't seem like a good long-term strategy. No other fix has been found.
Steps to Reproduce
yarn install
installsnormalize-url@6.0.1
Context
Safari is only ~5% of our users, but it still needs to be supported.
Your Environment
The text was updated successfully, but these errors were encountered: