Skip to content

fix: register viewer by uniq name instead of "component"#5312

Merged
juliusknorr merged 1 commit intomainfrom
fix/viewer-comopnent-registration
Jan 25, 2024
Merged

fix: register viewer by uniq name instead of "component"#5312
juliusknorr merged 1 commit intomainfrom
fix/viewer-comopnent-registration

Conversation

@ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jan 24, 2024

📝 Summary

  • Anonymous function in object property has a name of the property - component
  • This name is used for global viewer component registration
  • component is not a valid name, because it is a built-in name for Vue and it registers a global component
  • Using named function to have an explicit name

Probably should be handled by Viewer, but I haven't found any other app with async component registration. I guess it is not possible to use 2 viewers with dynamic anonymous components.

image

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

- Anonymous function in object property has a name of the property - component
- This name is used for global viewer component registration
- `component` is not a valid name, because it is a built-in name for Vue and it registers a global component
- Using named function to have an explicit name

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@juliusknorr juliusknorr merged commit eea6469 into main Jan 25, 2024
@juliusknorr juliusknorr deleted the fix/viewer-comopnent-registration branch January 25, 2024 07:50
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 25, 2024

@skjnldsv Do you think this should be handled by Viewer?
Currently Text is the only app that uses dynamic registration (according to the search in GitHub)

@skjnldsv
Copy link
Member

Just stumbled upon this.
We never claimed to support async components 🙈

image https://github.com/nextcloud/viewer/blob/c3e5c751b0decef5dbc103474208b6142f9deef9/src/services/Viewer.js#L11-L21

https://github.com/nextcloud/viewer/blob/d531abe602996292c8c0c45d0c525afcdc58bea6/src/views/Viewer.vue#L853-L854
I'm actually surprised this never blew up

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants