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

Skip valid_class check if no class defined #1656

Closed
TALlama opened this issue Apr 23, 2019 · 2 comments
Closed

Skip valid_class check if no class defined #1656

TALlama opened this issue Apr 23, 2019 · 2 comments
Labels

Comments

@TALlama
Copy link
Contributor

TALlama commented Apr 23, 2019

Environment

  • Ruby 2.6.2
  • Rails 5.1.7
  • Simple Form 4.1.0

Current behavior

We have a simple file input in our form:

<%= form.input :content, as: :file %>

As of 4.1, this is now a performance problem, because the file in question is stored in S3, and each time we render this form we now fetch the file from S3 to check if it's present, because simple_form wants to add the valid_class. In our case this is one of several such forms on the same page, which is now prohibitively slow.

We have defined valid_class: nil in our wrapper config, but simple_form still does the work to determine validity even if no class will be added if we are valid. We also marked the field as required: false in hopes that it being optional would allow it to be valid when it's not present, but that doesn't affect anything either.

This is very similar to #1596, except that only talks about getting rid of the CSS class, whereas I want to go a step further and avoid doing the (sometimes expensive) work that determines if a CSS class is necessary.

Expected behavior

If valid_class is nil (in the wrapper config or in the input options), no validity check should be performed.

TALlama added a commit to TALlama/simple_form that referenced this issue Apr 23, 2019
heartcombo#1656

Previously, we would check for validity all the time, even if the outcome of the check added no class. This meant that there was no way to opt out of the check in cases where it was expensive (if the file is stored in S3, in our example).

Now, we only check for validity if there is a valid_class defined to be added if the field is valid.
@marcelolx
Copy link

@TALlama I think this issue can be closed, right?

@feliperenan
Copy link
Collaborator

Oh yeah, it seems closed. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants