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

New 'negative lookbehind' regex breaks in IOS & Safari versions < 16.4 #10

Closed
4 tasks done
lllleonnnn opened this issue Aug 22, 2024 · 62 comments
Closed
4 tasks done
Labels
🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on

Comments

@lllleonnnn
Copy link

Initial checklist

Affected packages and versions

2.0.1

Link to runnable example

No response

Steps to reproduce

Run latest 2.0.1 on any Safari version below 16.4

Expected behavior

Expected behavior is that no errors should be thrown

Actual behavior

When using this as part of rendering markdown in React apps, will throw SyntaxError: Invalid regular expression: invalid group specifier name and crash application.

Affected runtime and version

'browser'@2.0.1

Affected package manager and version

No response

Affected OS and version

iOS <= 16.3

Build and bundle tools

No response

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Aug 22, 2024
@ChristianMurphy
Copy link
Member

Welcome @nowfred! 👋
This project targets ES2022, older versions of the spec are not part of the support matrix.

Safari 16.4 and below do not support ES2022 out of the box.
You can transpile/polyfill this with babel, take a look at https://github.com/slevithan/babel-plugin-transform-regex

Related to remarkjs/react-markdown#800 and remarkjs/react-markdown#822

@ChristianMurphy ChristianMurphy closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2024
@ChristianMurphy ChristianMurphy added the 🙋 no/question This does not need any changes label Aug 22, 2024

This comment has been minimized.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Aug 22, 2024
@lllleonnnn
Copy link
Author

Thank you @ChristianMurphy no worries - I personally appreciate finding issues like this when I'm debugging so wanted to leave for posterity. Appreciate your prompt reply and explanation.

@vktrl
Copy link

vktrl commented Oct 1, 2024

Welcome @nowfred! 👋 This project targets ES2022, older versions of the spec are not part of the support matrix.

Safari 16.4 and below do not support ES2022 out of the box. You can transpile/polyfill this with babel, take a look at https://github.com/slevithan/babel-plugin-transform-regex

Related to remarkjs/react-markdown#800 and remarkjs/react-markdown#822

@ChristianMurphy, please reconsider using a backwards-compatible regex or an option to specify one.

It can't be polyfilled and the babel transformer does not fix it.

Check the REPL and see that the pattern that's causing the problem is not being transformed: /(?<=^|\s|\p{P}|\p{S})([-.\w+]+)@([-\w]+(?:\.[-\w]+)+)/gu

For anyone else having this problem, I'm using pnpm to force compatible version for now:

// .pnpmfile.cjs

function readPackage(pkg, context) {
  if (pkg.name === 'mdast-util-gfm') {
    pkg.dependencies = {
      ...pkg.dependencies,
      'mdast-util-gfm-autolink-literal': '2.0.0',
    };
    context.log('mdast-util-gfm -> [email protected]');
  }

  return pkg;
}

module.exports = {
  hooks: {
    readPackage,
  },
};

@wooorm
Copy link
Member

wooorm commented Oct 1, 2024

Sad that that polyfill doesn’t seem to do lookbehinds.

“Backwards compatible” to what? The stone age? I don’t think it is reasonable to be compatible with ancient things.

There is of course some decent compatibility, which is outlined in the readme: https://github.com/syntax-tree/mdast-util-gfm-autolink-literal#compatibility. Sadly, Safari is more often than not, more recently, the place where things break first.

You can indeed use older versions if you want to.

@vktrl
Copy link

vktrl commented Oct 1, 2024

“Backwards compatible” to what? The stone age? I don’t think it is reasonable to be compatible with ancient things.

Safari 16.4 has only been out for 1.5 years. It's not from the stone age, and people are using it whether we like it or not.

I don't think it's unreasonable to ask for compatibility that goes back a little further than 2023. Especially if the solution is trivial and can't be polyfilled.

You can indeed use older versions if you want to.

I don't want to, but I'm forced to. This isn't something I don't have control over because it has to run client-side. In a perfect world people would keep their devices up to date, but alas.

@wooorm
Copy link
Member

wooorm commented Oct 1, 2024

Why do you think it’s trivial to solve?

You are of course right that the stone age was not 1.5 years ago.
However, you do understand that there is a point, somewhere, right?
There are also people that advocate the inverse of you: to not support old things.

When we cut releases, we drop support for old things. This is a breaking change that was communicated: https://github.com/syntax-tree/mdast-util-gfm-autolink-literal/releases.

@vktrl
Copy link

vktrl commented Oct 1, 2024

Why do you think it’s trivial to solve?

Because it's just one line in this commit.

You are of course right that the stone age was not 1.5 years ago. However, you do understand that there is a point, somewhere, right? There are also people that advocate the inverse of you: to not support old things.

There absolutely is a point somewhere, but is this really it? Breaking a recent browser with no workaround available because an improved regex pattern shaves off ~0.025ms when matching 5000 character strings?

@ChristianMurphy
Copy link
Member

There absolutely is a point somewhere, but is this really it?

It is

a recent browser

A browser version which is no longer supported by Apple

@ChristianMurphy
Copy link
Member

no workaround available

There are a number, you can hold on the older version of the package.
Try other transpiler/polyfills.
Or patch the old regex in if you want things to run slower https://github.com/ds300/patch-package#readme

@ChristianMurphy
Copy link
Member

My stance is pretty simple, if the world's richest company doesn't have time or money to support a version of their official and licensed product.
I don't have time to support it for free in my personal time.

@0xd8d
Copy link

0xd8d commented Oct 3, 2024

My stance is pretty simple, if the world's richest company doesn't have time or money to support a version of their official and licensed product. I don't have time to support it for free in my personal time.

Dismissing users on iOS < 16.4, which is still a fairly recent version, feels like an arrogant decision. It’s frustrating to see this issue brushed off when a simple regex tweak could easily prevent the crashes users are facing.

To make matters worse, this was a 2.0.0 to 2.0.1 update. A patch release should never introduce breaking changes. Dropping support for a significant chunk of users and telling developers to downgrade or deal with it isn’t just unreasonable, it’s irresponsible.

@wooorm
Copy link
Member

wooorm commented Oct 3, 2024

The breaking change happened in v2; nothing is brushed off, it was communicated; it’s not simple to change

@vktrl
Copy link

vktrl commented Oct 3, 2024

The breaking change happened in v2;

No, it happened in 2.0.1 with this commit as pointed out previously. 2.0.0 works fine.

it was communicated;

There's no mention of this in the readme or release notes.

it’s not simple to change

It is, but you've dug in and don't want to. It's just one line. I ran some benchmarks and the only tradeoff is shaving off fractions of milliseconds.

It's fine if you don't want to support Safari out of spite. It's your library and your choice. @ChristianMurphy was very straightforward about this and I got my answer.

I don't understand why you're gaslighting us over the facts though @wooorm?

@0xd8d
Copy link

0xd8d commented Oct 3, 2024

The breaking change happened in v2; nothing is brushed off, it was communicated; it’s not simple to change

It’s one thing to drop support for older browsers, but it’s another to ignore valid concerns when the fix is right in front of us. The fact that this issue popped up in 2.0.1, not in v2, shows this wasn’t a properly communicated breaking change. No one should have to comb through commits to figure out why a patch release suddenly breaks their app.

Saying the fix isn’t simple feels like an excuse when it’s a straightforward regex adjustment. We’re talking about preventing crashes for a significant number of users on Safari, and the performance hit is minimal at best.

@ChristianMurphy
Copy link
Member

I understand the frustration that many users have brought up in this thread, and I want to offer some clarity from my perspective.

First, it’s important to remember that the project's compatibility scope explicitly targets ES2022. The use of newer regex features like negative lookbehinds fits within that standard. Introducing features that align with the support matrix is not a breaking change and does not require special communication or break semantic versioning rules. Maintaining backward compatibility with older browsers, like Safari versions before 16.4, was never intended as a priority for this version.

The suggestions made, such as using tools like Babel or holding on to a prior version of the package, are not dismissive. They are practical options to meet the needs of projects requiring broader compatibility. I understand that polyfills and transpilers are not perfect solutions in every scenario, but they are part of the ecosystem's flexibility to bridge the gap between modern and legacy environments.

The comment about Apple no longer supporting Safari versions before 16.4 is significant. These versions are not receiving updates, which means users of those versions are already vulnerable to other issues. While I sympathize with the need to accommodate users restricted to older devices or browsers, it is also important for developers to focus efforts where there is active support.

Lastly, I want to make it clear that decisions about feature changes are not about spite or dismissiveness. They are choices made considering the larger technical roadmap and the constraints of developer time and maintenance complexity. Sometimes that means drawing a line where older environments are no longer supported.

The option to use an older version or a different configuration remains open for those needing it. Moving forward, embracing the latest standards is part of progressing in a landscape where technology evolves quickly. Thank you all for sharing your views. I respect the differing perspectives here, and I hope this helps explain the rationale behind the choices made for this project.

@0xd8d
Copy link

0xd8d commented Oct 3, 2024

I understand the frustration that many users have brought up in this thread, and I want to offer some clarity from my perspective.

First, it’s important to remember that the project's compatibility scope explicitly targets ES2022. The use of newer regex features like negative lookbehinds fits within that standard. Introducing features that align with the support matrix is not a breaking change and does not require special communication or break semantic versioning rules. Maintaining backward compatibility with older browsers, like Safari versions before 16.4, was never intended as a priority for this version.

The suggestions made, such as using tools like Babel or holding on to a prior version of the package, are not dismissive. They are practical options to meet the needs of projects requiring broader compatibility. I understand that polyfills and transpilers are not perfect solutions in every scenario, but they are part of the ecosystem's flexibility to bridge the gap between modern and legacy environments.

The comment about Apple no longer supporting Safari versions before 16.4 is significant. These versions are not receiving updates, which means users of those versions are already vulnerable to other issues. While I sympathize with the need to accommodate users restricted to older devices or browsers, it is also important for developers to focus efforts where there is active support.

Lastly, I want to make it clear that decisions about feature changes are not about spite or dismissiveness. They are choices made considering the larger technical roadmap and the constraints of developer time and maintenance complexity. Sometimes that means drawing a line where older environments are no longer supported.

The option to use an older version or a different configuration remains open for those needing it. Moving forward, embracing the latest standards is part of progressing in a landscape where technology evolves quickly. Thank you all for sharing your views. I respect the differing perspectives here, and I hope this helps explain the rationale behind the choices made for this project.

Yes, this project targets ES2022, but the reality is that this change broke things for a huge number of users still on Safari < 16.4. You can argue all day about technicalities and semantic versioning, but when the end result is crashes and broken apps, it’s a breaking change. Period. And in a patch release like 2.0.1, which developers expect to be stable, this kind of oversight isn’t just frustrating, it’s a major failure in communication and responsibility.

Telling developers to either roll back to an older version or use Babel isn’t a “solution.” It’s a cop-out. Not everyone has the flexibility to introduce complex build setups just to accommodate a change that could easily be fixed with a single line tweak. Why should the burden be on us to patch a simple regex issue that your team introduced in the first place?

And as for dismissing Safari < 16.4 because Apple doesn’t support it anymore, that’s a weak excuse. Just because Apple moves on doesn’t mean the rest of the world does, and pretending those users don’t exist doesn’t magically make the problem go away. We’re not talking about browsers from a decade ago, we’re talking about versions still actively used by millions. Writing them off like they don’t matter is irresponsible.

Your stance on “embracing the latest standards” feels more like stubbornness than progress. This isn’t about evolving technology, it’s about prioritizing the community that depends on your library. Developers trusted your package, and now they’re dealing with unnecessary crashes because you refuse to address an easily solvable issue.

The fix is trivial, the impact is massive, and the refusal to handle this properly is a clear signal that the user base isn’t being considered. If the goal is to alienate developers and break apps, you’re doing a great job. If not, this really needs to be addressed, not dismissed with empty justifications.

@ChristianMurphy
Copy link
Member

I appreciate your perspective and the time you've taken to lay out your concerns. Let me respond in kind, acknowledging both the technical realities and the broader considerations at play.

First, this project, like many others targeting modern JavaScript standards, follows a well-defined compatibility matrix. It’s true that users of Safari < 16.4 have experienced issues, but the decision to target ES2022 is not arbitrary. It's based on the desire to push forward and leverage features that improve performance, maintainability, and long-term viability of codebases. This benefits the broader community, millions of users who appreciate faster load times and more efficient code execution.

Regarding the patch update, it's worth remembering that "stable" doesn't always mean "unchanged forever." Stability in software is as much about keeping up with the standards as it is about consistency. We chose to implement a feature that aligns with ES2022, which is where the broader JavaScript ecosystem is headed. A move towards newer standards is essential for the health of the ecosystem. Introducing transpilers or polyfills to accommodate those browsers is not an unreasonable ask. It's a well-established practice in the development community. Adopters need to adapt to the standards or use the tools available, instead of expecting those moving forward to hold back for a few who haven’t kept pace.

You mentioned that the solution is trivial. But let’s be clear, what might look like a trivial regex tweak isn’t just about altering a line of code. It's about the implications for performance and maintainability for the vast majority of users. The current implementation provides meaningful gains, even if those gains seem small to you. However, when multiplied across millions of uses, these incremental improvements can significantly impact the efficiency of applications. Asking us to revert a valuable enhancement to accommodate older, unsupported environments is the very thing that holds back progress. We can't let decisions for the many be dictated by the few who are unwilling or unable to adapt their tools.

On the topic of maintaining backward compatibility, I have to say it's a bit disappointing to see developers who have never contributed to this project or even to open source in general become so vocal about demanding changes. Open source is built on collaboration and contribution. If this library truly matters to you, there are avenues for engaging constructively, whether by contributing code, proposing meaningful improvements, or even adding documentation to help others navigate these changes. Instead, I see repeated calls to "fix this trivial issue" without an offer to be part of the solution. Perhaps it is easier to criticize than to contribute.

As to those personal remarks about "stubbornness" or implying this stance is irresponsible, I would suggest considering the broader picture. The community relies on maintainers, many of whom do this work in their personal time, not just to maintain stability but also to drive innovation. If the richest companies out there have moved on from supporting these older environments, it's not a trivial ask for open-source maintainers to spend their limited time and resources filling in those gaps.

Lastly, I genuinely want what's best for developers using this library. You always have options, whether that means staying on an older version, using a transpiler, or even forking the library and making the changes yourself. Those are not dismissive responses. They are practical realities in the world of software development. The commitment to progress means making choices that sometimes leave legacy systems behind, but ultimately pave the way for a more robust, capable, and efficient ecosystem.

The message is clear: progress comes with change, and embracing that is a responsibility we all share.

@vktrl
Copy link

vktrl commented Oct 4, 2024

We can't let decisions for the many be dictated by the few who are unwilling or unable to adapt their tools.

If the richest companies out there have moved on from supporting these older environments, it's not a trivial ask for open-source maintainers to spend their limited time and resources filling in those gaps.

I agree with this sentiment when it comes to dev tooling, but we can't push updates to client devices. I did a quick check and it looks like about 5% of my users on iOS are affected by this.

It's weird to frame it like it's somehow supporting Apple and their shitty browser vs the actual real-world users who are using older devices for one reason or another. I try to use the latest standards possible, all in on ESM, etc, but I choose 5% of end-users being able to use my software over "embracing progress and change" every time.

First, this project, like many others targeting modern JavaScript standards, follows a well-defined compatibility matrix. It’s true that users of Safari < 16.4 have experienced issues, but the decision to target ES2022 is not arbitrary.

This has nothing to do with ES2022. It's a regex feature that was introduced in Node 8 and in Chrome in 2017. The real issue here is Safari being the modern IE and giving us another edge case to handle.

All solutions have tradeoffs:

a. Revert the change in the library and trade a little performance for compatibility.
b. Add an escape hatch to allow usage of the old regex for compatibility, increasing code complexity.
c. Have library consumers deal with it. Tradeoff being developers having to add more unmaintainable jank to their tooling until the market share drops low enough to warrant kicking out the last few who haven't upgraded.

Again - I'm fine with your decision, I have a fix in place. I understand that they are MY users not yours, and you have no obligation support them. What really annoys me is not acknowledging and misrepresenting what the issue is and what can be done about it. "Fuck Safari, you deal with it" is much less insulting to me, personally.

@graysonhicks
Copy link

@ChristianMurphy No? Then, please link me the line in 2.0.0 that caused this. I have read the thread, I understand the issue. The breaking change was introduced in 2.0.1.

@vktrl

This comment was marked as abuse.

@ChristianMurphy
Copy link
Member

There were breaking changes in 2.0.0 raising dependency and ES versions your build tools smoothed over those, but not the regex one.

@ChristianMurphy
Copy link
Member

insult towards all the developers (I see there's more) who've run into this issue due to this particularly jarring, there isn't an expectation of having to do testing for a x.x.1 release.

Your browser is explicitly outside the supported matrix for this package and has been for a while.
So yes, you do need to test.

And even if it were in matrix.
You are the maintainer of your own browser matrix and the steward for your own product. You need to test the code you are using is compatible for your users.

There’s no insult here, just an expectation you understand your responsibility and take competent steps to resolve it.

Some solutions are articulated here: #10 (comment)

To be extra clear, the compenent step is not attacking the open source maintainers who gave you free code and free support, because you feel inconvenienced.


A friendly reminder there is a code of conduct https://github.com/syntax-tree/.github/blob/main/code-of-conduct.md
A lot of frustrated folks are toeing the line of what is allowed.
Spamming or Trolling this thread, or issues as a whole, will lead to bans from accessing the support system.


If you have questions on how to do so the discussion forum is open https://github.com/orgs/syntax-tree/discussions/categories/q-a

I understand folks are confused and frustrated, and I empathize.
That doesn't change the support matrix for this module.

1395173231 added a commit to 1395173231/ChatGPT-Next-Web that referenced this issue Nov 24, 2024
BertKiv pushed a commit to BertKiv/LibreChat that referenced this issue Dec 10, 2024
…nny-avila#4574)

* fix: `mdast-util-gfm-autolink-literal` overriden to  due to syntax-tree/mdast-util-gfm-autolink-literal#10

* chore: non-change to force workflow

* Revert "chore: non-change to force workflow"

This reverts commit ba7c15d.

* chore: non-change to force workflow pt. 2

* chore: revert commit
@LiuAoYu
Copy link

LiuAoYu commented Dec 12, 2024

欢迎 @nowfred!👋 该项目以 ES2022 为目标,旧版本规范不属于支持矩阵的一部分。

Safari 16.4 及以下版本不支持 ES2022。 您可以使用 babel 进行转译/polyfill,请查看https://github.com/levithan/babel-plugin-transform-regex

与remarkjs/react-markdown#800remarkjs/react-markdown#822相关

I followed the 'babel plugin transfer form regex' tutorial to configure it, but it doesn't work. Is there anything to pay attention to? What should I do?

@ChristianMurphy
Copy link
Member

@LiuAoYu It needs to swap in xregex, along with this xregex add on slevithan/xregexp#77

That said, if you want to get setup quickly, locking the version of this package to 1.x.x or 2.0.0 will use the old regex.
Or if you want to continue to get other updates and fixes, consider using patch-package https://github.com/ds300/patch-package#readme to swap a different regex into newer versions of the package.

@wellssu0
Copy link

I used resolutions to lock the version to 2.0.0.

{
 "resolutions": {
    "mdast-util-gfm-autolink-literal": "2.0.0"
  }
}

@hiepxanh
Copy link

As I see through all of this, there is 2 option:

  1. lock version
  2. try to replace the regex

I thinking about simple solution, the library can have 2 object of regex pattern, using old version if regex look ahead is not support or using new version if look ahead support when init the code 🤔

this will not reduce performance since we can decide which kind of regex we should use before load it.

@wooorm
Copy link
Member

wooorm commented Dec 13, 2024

See my earlier comments, #10 (comment), #10 (comment). Things will keep on breaking on old browsers when you use the latest tools that have Node 16 compatibility ranges. Different regex will be a performance problem for older browser. Feel free to investigate a patch that convincingly solves the things mentions in this thread in a PR, but I have strong doubts.

@hiepxanh hiepxanh mentioned this issue Dec 14, 2024
4 tasks
@hiepxanh
Copy link

See my earlier comments, #10 (comment), #10 (comment). Things will keep on breaking on old browsers when you use the latest tools that have Node 16 compatibility ranges. Different regex will be a performance problem for older browser. Feel free to investigate a patch that convincingly solves the things mentions in this thread in a PR, but I have strong doubts.

I understand your concern, I'll try my best to not failure you sir 😸

@skyriverbend
Copy link

skyriverbend commented Jan 7, 2025

My app broke for iPad (9th gen) users as soon as I implemented remark-gfm, which relies on this package. It was a shitty experience for everyone.

This ISN'T industry standard. This is the first time I've installed a package that broke support for such recent browser. And I've installed a lot of packages.

At the very least, please have the maintainers of remark-gfm put a warning at the top of the readme stating it's not compatible with a version of Safari that is only 2 years old.

@skyriverbend
Copy link

skyriverbend commented Jan 7, 2025

Here's an example of industry standard behavior:

unjs/ufo#226

They fix the issue and at the same time mention it's temporary because most browsers support 'negative lookbehind'.

The fact that 30% of your entire Github issue history is reporting this issue, on top of all the resistance in this thread... I would hope these are indicators you might take seriously.

@wooorm
Copy link
Member

wooorm commented Jan 7, 2025

Please read things before you — that patch does not work here.
You know little of the issues spread out across the 600 projects maintained here.

@skyriverbend
Copy link

Can the maintainers point to a single real world production web app that doesn't support Safari 16.4?

I would wager that Safari 16.4 support would be a requirement for the vast majority of production apps. You're shipping code that breaks in production. It would be fine if you put a big notice at the top of all the readme's depending on this library, but you haven't. So understandably people are getting upset because you're breaking their apps and pretending like it's their fault. Sure ban us if you want. But you're shipping code that breaks in production and then blaming users for not having read your obscure browser matrix that's buried deep within an obscure repository. That sucks bad. Please at the very least put a bold warning at the top of all the readme's that depend on this library.

@wooorm
Copy link
Member

wooorm commented Jan 7, 2025

you should read the compatibility section in the readme and you should read the license. You should also pay the people that build your infrastructure. Finally, you should use lock files and test your apps.

@skyriverbend
Copy link

I did read the compatibility section. There's no browser matrix or anything indicating incompatibility with a recent version of safari:

I've locked to 2.0.0 now. But you have to admit this is a breaking change.

Also I've sponsored open source contributors for thousands of dollars. I'm genuinely grateful for all the work you've done. I'm just trying to raise a voice here, because I'm sure this is going to continue to cause bugs for thousands more users if you don't actually take it seriously.

@wooorm
Copy link
Member

wooorm commented Jan 7, 2025

  • the inclusion of Node and absence of iOS in a compat matrix gives you enough info
  • no, I do not think this is a semver breaking change, iOS is explicitly not covered in our compatibility matrix, but Node is: there will always be engines that are not tested by us, that do not support things that we use. Safari will keep on breaking and breaking because they do not invest in the web, and do not implement features that landed 8 years ago in v8
  • it is sad when things break and we often prevent things breaking, but we cannot reasonably prevent this break, because it is a trade off between things, compat + performance
  • while you feel unheard you are not, while you feel treated unseriously, you are not

@neil-morrison44
Copy link

Since your compatibility section doesn’t mention anything other than node, iOS is implicitly not covered by it, if you want to explicitly not cover it (or, presumably any other web browser) you need to actually mention it.

Did the PR to patch this go anywhere?

@skyriverbend
Copy link

Literally as you sit here defending your actions, an open source app with 20k stars and hundreds of thousands of users just reported having their app randomly break on users.

danny-avila/LibreChat#5204

How can you justify this? Especially considering the comments in this thread indicate that the regex provided only a marginal performance improvement. You're cutting out 6% of users on the internet with this tiny regex line that you refuse to change.

Continuing to ignore this issue is causing active harm.

@wooorm
Copy link
Member

wooorm commented Jan 7, 2025

if you want to explicitly not cover it [...] you need to actually mention it.

That’s not what I want.

marginal

6%

These numbers are incorrect. Please do not misrepresent.

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Jan 7, 2025

Since your compatibility section doesn’t mention anything other than node, iOS is implicitly not covered by it, if you want to explicitly not cover it (or, presumably any other web browser) you need to actually mention it.

@neil-morrison44 that isn't how support matrices work.
Everything listed is explicitly supported, everything not listed is explicitly not supported.

Did the PR to patch this go anywhere?

You can read for yourself #14

this thread indicate that the regex provided only a marginal performance improvement

@skyriverbend It is significant

How can you justify this?

The entitlement and narcissism in your comments is stunning.
Different projects have different support matrices, the world does not revolve around you.
There is nothing to justify, older browsers are supported by older versions of the library.
You can continue to use the previous version, or you can encourage your users to upgrade.

You're cutting out 6% of users on the internet with this tiny regex line that you refuse to change.

No, that number is both inaccurately over represented, and for the actual small fraction of users, they can use the old version until they are ready to upgrade.
This backwards IE style mindset is what causes broken slow packages, Marvin Hagemeister did a great series in performance https://marvinh.dev/blog/speeding-up-javascript-ecosystem-part-6/
Polyfills and excessive de-optimization for legacy browser support can be significant bottlenecks, hurting everyone on the internet, not a small fraction.

@neil-morrison44
Copy link

Assuming that https://github.com/remarkjs/remark-gfm?tab=readme-ov-file#compatibility is the extent of where you talk about compatibility it isn't a matrix. You are explicitly supporting maintained versions of node, and you could read into that that you're implicitly not supporting anything else.

While I don't endorse the tone / content of the other recent commenter, rejecting a PR which would have resolved this issue for you and users of your library baffles me. The pragmatic approach would surely to have been to accept it until usage of that old version of Safari drops off completely and you can remove it without risk of getting all these annoying github comments from people who find this issue after having to dig through their compiled JS to find the error that users of old versions of Safari were reporting just like myself and LibreChat did.

I don't expect you to do work for free, but I would expect some degree of good stewardship of your open source project.

@ChristianMurphy
Copy link
Member

is the extent of where you talk about compatibility it isn't a matrix

It is a matrix, we support LTS versions of Node https://nodejs.org/en/about/previous-releases

You are explicitly supporting maintained versions of node, and you could read into that that you're implicitly not supporting anything else.

Yes, that is how matrices work, anything not explicitly listed, is explicitly not included.
I would like to think that the project does not need to explain what a support matrix is in each of the 600 repositories to avoid pedantic commenters.

The pragmatic approach would surely to have been to accept it until usage of that old version of Safari drops off completely

Pinning the version is both pragmatic and principled.
Safari 16.4 already has fallen off, you can of course choose to support it, and the older version is there to allow you to do so.
Don't force the rest of the internet to have either a bloated package or bad performance to support your legacy matrix, that would be bad open source stewardship.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests