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

Add bundled analytics support in Inspector #616

Merged
merged 1 commit into from
Oct 16, 2022

Conversation

zikaari
Copy link
Contributor

@zikaari zikaari commented Oct 6, 2022

[x] I've included a changeset (psst. run yarn changeset. Read about changesets here).

Corresponding Inspector PR - https://github.com/segmentio/segment-inspector/pull/24

@zikaari zikaari requested review from silesky and chrisradek October 6, 2022 19:34
@changeset-bot
Copy link

changeset-bot bot commented Oct 6, 2022

🦋 Changeset detected

Latest commit: 58c6081

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@segment/analytics-next Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@zikaari zikaari force-pushed the bundled_analytics_inspector branch from f682dae to 2c828dd Compare October 7, 2022 19:02
@silesky silesky enabled auto-merge (squash) October 7, 2022 19:13
@zikaari zikaari force-pushed the bundled_analytics_inspector branch 3 times, most recently from 67b324d to 707fd25 Compare October 7, 2022 19:34
@silesky
Copy link
Contributor

silesky commented Oct 7, 2022

I think the build is causing a typescript error because of this line, which is conflicting with ambient declaration merging:
image

This is the correct declaration:

declare global {
  interface Window {
    analytics: AnalyticsSnippet
  }

All this is complicated by the fact that there's a circular dependency here with analytics-next and '@segment/inspector-webext'. Publishing inspector-webext's types as a separate dependency or just removing that dependency altogether from analytics-next might be a good idea (and doing the typing locally).

@zikaari zikaari force-pushed the bundled_analytics_inspector branch 3 times, most recently from b43e1e4 to 63e412a Compare October 14, 2022 21:55
@zikaari zikaari force-pushed the bundled_analytics_inspector branch from 63e412a to 58c6081 Compare October 14, 2022 22:01
@silesky silesky merged commit 1d6c22d into master Oct 16, 2022
@silesky silesky deleted the bundled_analytics_inspector branch October 16, 2022 16:41
@github-actions github-actions bot mentioned this pull request Oct 16, 2022
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