Skip to content

Upgrade ViewComponent dependencies#8469

Merged
aduth merged 3 commits intomainfrom
aduth-upgrade-view-component
May 26, 2023
Merged

Upgrade ViewComponent dependencies#8469
aduth merged 3 commits intomainfrom
aduth-upgrade-view-component

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented May 23, 2023

🛠 Summary of changes

Upgrades ViewComponent and Lookbook to their latest respective versions, both of which are major upgrades:

  • ViewComponent: 2.82.0 ➡️ 3.0.0
  • Lookbook: 1.5.3 ➡️ 2.0.3

Notable breaking changes encountered:

  • Trying to reference translation helpers in component constructor results in an error (see changes to PasswordToggleComponent)
  • controller and request were removed from spec helpers, and were updated to equivalent vc_test_controller and vc_test_request.
  • Slot API changes result in slot content rendering requiring a with_ prefix

📜 Testing Plan

@aduth
Copy link
Contributor Author

aduth commented May 23, 2023

Based on spec failures, need to fix PasswordConfirmationComponent for similar reason as mentioned for translations in constructor as in PasswordToggleComponent. Will do that after merging #8466 to take advantage of specs being added for the component there.

@aduth aduth force-pushed the aduth-upgrade-view-component branch from aaaa99b to ce78d50 Compare May 24, 2023 19:53
@aduth
Copy link
Contributor Author

aduth commented May 24, 2023

Based on spec failures, need to fix PasswordConfirmationComponent for similar reason as mentioned for translations in constructor as in PasswordToggleComponent. Will do that after merging #8466 to take advantage of specs being added for the component there.

Fixed in rebased ce78d50.

I'm noticing a strange (seemingly harmless) Rack lint error when viewing component preview pages (example page):

Rack::Lint::LintError: Status must be >=100 seen as integer

It's followed by a 500 for a /components/cable/ request:

 "GET /components/cable/?uid=1684957888289 HTTP/1.1" 500 1637 0.2352

One of the changes in the Lookbook 2.0 upgrade related to actioncable dependency (docs). It was mentioned as optional, but given the 500, I'm curious how optional it really is. In any case, since Propshaft (#8457) requires one of the two mentioned dependencies (listen 🧚 ), I might try to see if the errors still happen after merging that pull request, and whether it's necessary to pull in actioncable as well.

aduth added 2 commits May 25, 2023 09:57
changelog: Internal, Dependencies, Update UI component dependencies to latest versions
@aduth aduth force-pushed the aduth-upgrade-view-component branch from ce78d50 to 29e3a17 Compare May 25, 2023 14:09
@aduth
Copy link
Contributor Author

aduth commented May 25, 2023

I might try to see if the errors still happen after merging that pull request, and whether it's necessary to pull in actioncable as well.

I tried again after merging #8457 and it still happens. Troubleshooting more, it seems to be related to ActionCable responding with a -1 status code as part of its handling for Websockets, which Rack::Lint doesn't take kindly to. Generally, I think this is harmless aside from the terminal output, and affects only local development, and only on the /components/ routes, so I don't really feel too compelled to try to find a workaround.

See: rails/rails#26179

@aduth aduth merged commit 1ddc3e3 into main May 26, 2023
@aduth aduth deleted the aduth-upgrade-view-component branch May 26, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants