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

Using RegExp replace loop in isHidden #109

Merged
merged 3 commits into from May 15, 2020
Merged

Conversation

Jiasm
Copy link
Contributor

@Jiasm Jiasm commented Aug 23, 2018

I think RegExp will faster than loop.

Copy link
Member

@3imed-jaberi 3imed-jaberi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀, this my nano suggestion !!

function isHidden (root, path) {
  const hiddenFileReg = new RegExp(`${sep}\\.`)
  path = path.substr(root.length)

  return hiddenFileReg.test(path)
}

@Jiasm
Copy link
Contributor Author

Jiasm commented May 15, 2020

@3imed-jaberi sep is a constant, so i define hiddenFileReg in module scope.😄

@niftylettuce niftylettuce merged commit 8330028 into koajs:master May 15, 2020
niftylettuce added a commit that referenced this pull request May 15, 2020
niftylettuce added a commit that referenced this pull request May 15, 2020
@niftylettuce
Copy link
Contributor

I'm reverting this. Generally RegExp is unsafe in JavaScript, and for loops are incredibly fast.

Without a performance test nor benchmark, I cannot merge this. Sorry for the accident merge.

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.

None yet

3 participants