-
Notifications
You must be signed in to change notification settings - Fork 5
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
Investigate the performance implications using regex for replace #1
Comments
In the end it is easier to use regex instead of a manual algorithm. With regex you use less characters (which can help porting this into the browser) and also we make sure that the behavior is well tested and consistent. Bottom of line: This lib only abstracts the complexity of handling replace using Regular Expression in JavaScript. See https://github.com/FagnerMartinsBrack/str-replace/releases/tag/v0.0.4 |
Exactly my question when reading about this. Probably worth mentioning the RegExp implementation in the README somewhere IMO. |
Wouldn't that be leaking the internals to the consumer? Why would someone find useful knowing how it is done internally? Maybe for trust purposes? |
Performance, and perhaps security. There are well-known vulnerabilities with RegExp including DOS, as well as performance characteristics (for better or worse). The internals are exposed in the open source code already ;) |
The fact the internals are exposed in the source is irrelevant, one should never rely on the library internals for these reasons. Basically, most of the consumers of an API does not inspect the open source internals, they rely on the documentation to decide how the API should be used. That comes back to my question: what would be the value for most of the consumers of an API to know how the library works internally? If they want to know it is just a matter of inspecting the source anyway, I see no reason to explicitly document it. Also, in this case, what is the relationship of performance or security? It's not like performance or security is going to be fixed by documenting the underlying RegExp implementation. |
The article you reference is all about internal APIs, not code/statements, and I totally agree. (Undocumented API's can be broken without remorse.)
Of course it's your library and you can document whatever you (don't) want. If it were me, I'd call this DSL sugar around RegExp, and document it in a README so other developers could make well-informed decisions to use or not. YMMV. |
@crazy4groovy Thanks for the feedback, created an issue for that, see #3. |
Investigate and document if there is a considerable change in execution time for the major browsers using regex for the replace/replaceAll operations. The problem with using regex is that it will be a lot of work to escape the proper characters. But if it means a considerable benefit ... so be it.
The text was updated successfully, but these errors were encountered: