laminas / laminas-validator

Validation classes for a wide range of domains, and the ability to chain validators to create complex validation criteria
https://docs.laminas.dev/laminas-validator/
BSD 3-Clause "New" or "Revised" License
132 stars 57 forks source link

Migrate to `Laminas\Translator` #301

Closed gsteel closed 4 months ago

gsteel commented 4 months ago
Q A
BC Break yes

Description

This patch drops all usage of Laminas\I18n\* symbols and requires Laminas\Translator for src

All validator specific translation classes are gone, i.e. Laminas\Validator\Translator\TranslatorInterface and friends because there's no need for them.

This theoretically opens the door for users to use alternative translation libs, so long as they implement Laminas\Translator…

Most changes here stem from alterations to the TranslatorAwareInterface which has received native parameter and return types, and, the removal of fluent returns.

There is more work to do to clean up AbstractValidator and also to decide what to do about the TranslatorAwareInterface - we will likely keep it (?), but I'm keen to at least drop the getters.

froschdesign commented 4 months ago

…and also to decide what to do about the TranslatorAwareInterface - we will likely keep it (?)…

laminas-validator should not provide this interface or any other interface for translations.

weierophinney commented 4 months ago

Oof.

It's been a while since I touched this component, and I'd forgotten entirely that it had translation features... and even what they were for.

They're for translating error messages.

Honestly, this feels like something that shouldn't even be part of this component. It's related to view/display logic, and that would be where I'd expect to translate the error messages, using something like this:

echo $translator->translate($validator->error(), $textDomain);

Doing it in the validator feels like a violation of SRP.

Since we'd be changing this for v3 anyways... maybe we should consider removing the functionality entirely?

gsteel commented 4 months ago

I figured that dropping translation entirely would be a BC break too far, but I'd be delighted to rip out the whole thing!

froschdesign commented 4 months ago

I don't know if anyone uses translations in APIs, because this would be eliminated if only input-filters were used. With laminas-form there should be no problem because the view helper FormElementErrors already translates.

gsteel commented 4 months ago

302 replaces this PR with a single commit ripping out the lot

boesing commented 4 months ago

I don't know if anyone uses translations in APIs, because this would be eliminated if only input-filters were used.

With laminas-form there should be no problem because the view helper FormElementErrors already translates.

at my old job, we definitely used translations for validator messages in form context which were then returned as json to the clients. So it is in use for sure somewhere. but thats just a sidenote.

froschdesign commented 4 months ago

@boesing

we definitely used translations for validator messages in form context which were then returned as json to the clients.

Correct the Form, Fieldset and Element classes itself does not translate, only the view helpers.

gsteel commented 4 months ago

Just to bring in some discussion from Slack…

@weierophinney, @froschdesign and @gsteel discussed the alternative of removing translator functionality completely for v3 (implemented in #302) - the decision was made to merge this PR and retain translator functionality for eventual removal in v4.

The longer term plan for v4 is stateless, immutable validators that will return a result object containing status and error messages etc - this is where a different translation solution will be required.