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

Consider renaming injection keys to avoid collisions #6

Open
IsraelOrtuno opened this issue Nov 16, 2022 · 6 comments
Open

Consider renaming injection keys to avoid collisions #6

IsraelOrtuno opened this issue Nov 16, 2022 · 6 comments

Comments

@IsraelOrtuno
Copy link

Hello! Thanks for the package, definitely useful for a quick installation in a Vue/Nuxt app. I found something that may be quite tricky to deal with and is the fact that the package uses Vue 3 provide to expose its api so it can be used anywhere. This is quite nice BUT the key names for these functions to be injected are too generic and are very likely to collision with other. I would suggest adding a prefix to those like tawk* (whatever other pattern may work too):

    const minimize = inject('tawkMinimize');
    const onLoad = inject('tawkOnLoad');
    const toggle = inject('tawkToggle');

Specially the ones mentioned in the example are way to generic and chances are that these names may already exist in an application. I am aware this may be a breaking change but will definitely help users to use the package and prevent unexpected behaviours so it could maybe be released in a v2 release.

@IsraelOrtuno
Copy link
Author

IsraelOrtuno commented Nov 16, 2022

Extracting the actions to an external module may be a good way to go to so the package could keep the current behaviour and add a more robust one without breaking changes:

import { onLoad, toggle, minimize } from 'tawk/actions'

Happy to help with a PR here if maintainers consider this approach.

@IsraelOrtuno
Copy link
Author

No reply in months

@chrisspiegl
Copy link

This would be incredibly important IMO!

@jaoaustero
Copy link
Contributor

@IsraelOrtuno Thank you for submitting this issue, we would like to apologize for having a delay
to look at this. We will provide an update within this week.

@chrisspiegl Yes this would be fix right away

@jaoaustero jaoaustero reopened this Oct 17, 2023
@pixelpaulaus
Copy link

yeah this is a big oversight using such generic names

@chreniuc
Copy link

@jaoaustero any news on this?

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

No branches or pull requests

5 participants