Skip to content

Conversation

hiepxanh
Copy link

@hiepxanh hiepxanh commented Dec 16, 2024

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

as we discuss about "negative lookbehind" not working on old version safari in this issue: #10
I try to update it:

  1. fix windows issue while runing on fresh test
  2. add a check if current system support:
function transformGfmAutolinkLiterals(tree) {
  if (regexSupport.lookbehind) {
    modernAutolinkTransform(tree)
  } else {
    legacyAutolinkTransform(tree)
  }
}

the test is simple and running before we do anything else:

// Regex support detector
const regexSupport = {
  lookbehind: (() => {
    try {
      // Using regex literal instead of RegExp constructor
      ;/(?<=x)/.test('x')
      return true
    } catch {
      return false
    }
  })()
}

the result is fine:

The before change test:
before

The after change test:
after with node 22

I also try to install nodejs 16 but it not work, so I realize I cannot use many new syntax since nodejs 16 not support. Then I understand why you want to skip support.

I also try to build, to replace my current code to test on my ios, but the node_modules not found the autolink-literal since it is very deep link. I will try to test futher on ios 16.

I hope you can give me some thought about my idea. I don't mind if it useless and should not be merge, since I'm not have many experience. I would like to here what you think.

Thank you <3

@github-actions github-actions bot added the 👋 phase/new Post is being triaged automatically label Dec 16, 2024

This comment has been minimized.

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

wooorm commented Jan 2, 2025

Appreciate you trying your hand at this!

CI is failing. That could be solved.

But importantly, in the other thread, I linked to and directly mentioned that “Different regex will be a performance problem for older browser”. This is important. I meant that as a blocker.

Your PR is essentially the same as for people who want to use old versions, to also use old versions. They can do that if they want/have to.

@hiepxanh
Copy link
Author

hiepxanh commented Jan 3, 2025

Happy new year 2025! <3 Thanks for the feedback!

Regarding the regex concern and the linked thread, I understand your point about performance on older browsers. That’s a valid blocker, I see your perspective. My intent was to provide a balance, but I’ll rethink the approach to align more closely with the broader goals of the project.

Thanks again for flagging these points! I understand now and we should keep your ways as it is the best as it could. Anyway, I love your plugin and all your ecosystem you build, really appreciate this, thank you so much

@hiepxanh hiepxanh closed this Jan 3, 2025

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Jan 3, 2025

Happy new year Hiep! :)

@wooorm wooorm added the 🙅 no/wontfix This is not (enough of) an issue for this project label Jan 3, 2025
@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 Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging this pull request may close these issues.

2 participants