Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
composer.json should reflect that react requires ext-filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@staabm
This ext. is since PHP 5.2 enabled by default. http://php.net/manual/en/filter.installation.php
I see many methods from multiple PHP ext.s used in react and no entry in composer.json for them.
What is the difference by ext-filter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which extensions does React use that aren't declared?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cboden f.x. PCRE (for preg_replace) is like Filter enabled per default to.
Is it maybe better to preg_match both instead of using filter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RafaelKa you neverthelesss should declar it, because a lot of people - especially on production servers - use non-default builds (``--disable-all`) which are stripped down to the very least for performance and security reasons.
@cboden from within this code-window I would say e.g.
ext-pcreThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, all dependencies should explicitly be defined (and kept to a minimum for obvious reasons). Though in this case, have you checked PHP's installation instructions? (http://php.net/manual/en/pcre.installation.php and http://php.net/manual/en/filter.installation.php)
None that I'm aware of. It anybody happens to spot one, please file an issue or even better a PR in the relevant component :-)