-
Notifications
You must be signed in to change notification settings - Fork 440
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
Add strict helpers #1987
Open
reeganviljoen
wants to merge
25
commits into
ViewComponent:introduce-component-local-config
Choose a base branch
from
reeganviljoen:rv-add-strict-helpers
base: introduce-component-local-config
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add strict helpers #1987
reeganviljoen
wants to merge
25
commits into
ViewComponent:introduce-component-local-config
from
reeganviljoen:rv-add-strict-helpers
Commits on Oct 8, 2024
-
Use ActiveSupport::Configurable to cascade config
ViewComponent::Base itself will always use the defaults. Any classes inheriting from ViewComponent::Base will cascade config down using InheritableOptions, only specifying any overrides. Generate options being on their own "layer of config is currently unresolved - it might be that config needs to be a new object inheriting from InheritableOptions that has method accessors for everything in that namespace. This was initially written to support extracting the incoming strict_helpers_enabled? option, but applies to everything.
Configuration menu - View commit details
-
Copy full SHA for 71c23c6 - Browse repository at this point
Copy the full SHA 71c23c6View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4b4ba76 - Browse repository at this point
Copy the full SHA 4b4ba76View commit details -
Make GlobalConfig a proxy for Rails app config or ViewComponent base …
…config as necessary
Configuration menu - View commit details
-
Copy full SHA for 0ba800b - Browse repository at this point
Copy the full SHA 0ba800bView commit details -
Configuration menu - View commit details
-
Copy full SHA for a655192 - Browse repository at this point
Copy the full SHA a655192View commit details -
Configuration menu - View commit details
-
Copy full SHA for f56832e - Browse repository at this point
Copy the full SHA f56832eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 2531907 - Browse repository at this point
Copy the full SHA 2531907View commit details -
Configuration menu - View commit details
-
Copy full SHA for 1f40e96 - Browse repository at this point
Copy the full SHA 1f40e96View commit details -
Remove component-local config for now
No settings actually use it, but this would be the way to introduce it once we come to need it.
Configuration menu - View commit details
-
Copy full SHA for 4c89cbc - Browse repository at this point
Copy the full SHA 4c89cbcView commit details -
Configuration menu - View commit details
-
Copy full SHA for 2c0daf6 - Browse repository at this point
Copy the full SHA 2c0daf6View commit details -
Configuration menu - View commit details
-
Copy full SHA for 161cee3 - Browse repository at this point
Copy the full SHA 161cee3View commit details -
Configuration menu - View commit details
-
Copy full SHA for caa79f3 - Browse repository at this point
Copy the full SHA caa79f3View commit details -
Configuration menu - View commit details
-
Copy full SHA for ab59bdd - Browse repository at this point
Copy the full SHA ab59bddView commit details -
Configuration menu - View commit details
-
Copy full SHA for 0ada155 - Browse repository at this point
Copy the full SHA 0ada155View commit details -
Configuration menu - View commit details
-
Copy full SHA for 6508f28 - Browse repository at this point
Copy the full SHA 6508f28View commit details -
Configuration menu - View commit details
-
Copy full SHA for b292de5 - Browse repository at this point
Copy the full SHA b292de5View commit details -
Configuration menu - View commit details
-
Copy full SHA for 380fe22 - Browse repository at this point
Copy the full SHA 380fe22View commit details -
Configuration menu - View commit details
-
Copy full SHA for fca14b1 - Browse repository at this point
Copy the full SHA fca14b1View commit details -
Configuration menu - View commit details
-
Copy full SHA for cb73965 - Browse repository at this point
Copy the full SHA cb73965View commit details -
Configuration menu - View commit details
-
Copy full SHA for 6d4bed1 - Browse repository at this point
Copy the full SHA 6d4bed1View commit details -
Configuration menu - View commit details
-
Copy full SHA for a4a7ed9 - Browse repository at this point
Copy the full SHA a4a7ed9View commit details
Commits on Oct 11, 2024
-
Fix duplicate template error bug (ViewComponent#2121)
* Fix duplicate template error bug This change fixes the bug reported where a component will report that it has both a template and a `call` method defined despite only having an HTML template. I was able to reproduce this bug by pulling in Avo (like reported in the initial bug report) and running `vegeta` (to trigger parallel requests since I had a hunch this was a race condition): ``` echo "GET http://localhost:3000/avo/resources/cars/1" | vegeta attack -duration=2s ``` This reliably reproduced the error: ``` ActionView::Template::Error (Template file and inline render method found for Avo::CoverPhotoComponent. There can only be a template file or inline render method per component. Template file and inline render method found for variant '' in Avo::CoverPhotoComponent. There can only be a template file or inline render method per variant. Templates: ``` I added _a lot_ of debug logs to understand the race condition that I thought was occurring, and realized that multiple threads are calling `gather_templates` _and_ mutating the `@templates` array at the same.When looking at the old compiler code and realized that this likely isn't new behavior. This led the investigation towards how we collect and surface errors or otherwise might modify templates. It turns out there's a difference in the new and old compiler code after the refactor: ```ruby \# old def template_errors @__vc_template_errors ||= \# new def gather_template_errors(raise_errors) errors = [] ``` _We're not memoizing the errors like we used to_. This is more correct behavior, but explains how a race condition would make this error case much more difficult to occur in older versions of the compiler. This change brings us back to the old behavior by memoizing the errors we collect in `gather_template_errors` but the `@templates` ivar is still being mutated. I don't want to change _too much_ in this PR, but a subsequent change might be wrapping the entire `compile` method in the `redefinition_lock` (and renaming it to `compile_lock`) to avoid similar issues in the future. I did not include a test because it's dfficult to reproduce this race condition reliably in the test environment. I believe this _should_ close out ViewComponent#2114 * Fix tests, raise errors for compiled components with errors * There isn't always a default template when there are errors. * Make standard happy ugh * Remove debug code * Fail compilation when errors present * Revert default template change * Add informative comment * Add changelog --------- Co-authored-by: Joel Hawksley <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 5da6b23 - Browse repository at this point
Copy the full SHA 5da6b23View commit details -
Configuration menu - View commit details
-
Copy full SHA for 93534b1 - Browse repository at this point
Copy the full SHA 93534b1View commit details -
Configuration menu - View commit details
-
Copy full SHA for 0470809 - Browse repository at this point
Copy the full SHA 0470809View commit details -
Configuration menu - View commit details
-
Copy full SHA for b7a6ad0 - Browse repository at this point
Copy the full SHA b7a6ad0View commit details -
Configuration menu - View commit details
-
Copy full SHA for dbfc516 - Browse repository at this point
Copy the full SHA dbfc516View commit details
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.