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

When API Key is undefined it builds invalid CDN url #2

Open
lsemerini opened this issue May 28, 2020 · 2 comments
Open

When API Key is undefined it builds invalid CDN url #2

lsemerini opened this issue May 28, 2020 · 2 comments
Labels
good first issue Good for newcomers

Comments

@lsemerini
Copy link

This is a great package - thank you!

One thing I noticed is that if the apiKey is undefined, it still builds the CDN url with apiKey undefined.

Maybe an enhancement would be to prevent it from working when apiKey isnt passed, so the user doesn't need to validate if the api key is set or not.

@anthonyshort anthonyshort added the good first issue Good for newcomers label May 28, 2020
@anthonyshort
Copy link
Contributor

Hey @lsemerini! Great suggestion. If you want to open a PR for this that would be great. You can do this by adding a check here and returning if apiKey isn't set. This will never load the Segment library, but it won't throw an error.

We'd probably need to log a warning in this case.

@lsemerini
Copy link
Author

I'll see if I can give it a shot.

Another thing is that when you add this check and later try to invoke useSegment() it will throw an error.

I had to do something ugly on my code so it failed silently and to avoid multiple conditionals afterwards.. like this:

const SegmentProviderWrapper: React.FC<SegmentProviderWrapperProps> = ({ children, apiKey }) => {
  if (!apiKey) {
    console.warn(
      'You are using SegmentProvider without an apiKey - Your tracking functions will be mocked by SegmentProviderWrapper',
    );
  }
  return apiKey ? <SegmentProvider apiKey={apiKey}>{children}</SegmentProvider> : children;
};

export const useSegment = () => {
  const analytics = useContext(SegmentContext);
  if (!analytics) {
    // fail silently if segment isnt initialzied properly instead of throw errors
    // when implementing a function we should always test that the fallback is available too
    return {
      initialize: (props: any) => {},
      identify: (props: any) => {},
      page: (props: any) => {},
      track: (props: any) => {}, // etcetera
    };
  }
  return analytics;
};

This way I can still call analytics.track in my code without checking whether is available or not. And I can call analytics = useSegment() even though the provider wasn't added higher up in the tree.

@lsemerini lsemerini reopened this May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Development

No branches or pull requests

2 participants