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

fix: avoid node-based sdk-utils in default export for package #125

Merged

Conversation

djones
Copy link
Contributor

@djones djones commented Apr 6, 2019

@percy/agent is sometimes imported like this in our SDKs:

import PercyAgent from '@percy/agent'

At which point the default export is imported. This export is intended to run in a browser and everything in the src/percy-agent-client directory is built to expect this.

As we've built out our SDKs we found that @percy/agent is also quite useful for centralizing SDK utility functions. We previously had been distributing these util functions through the default export, so you can go:

import { agentJsFilename, isAgentRunning, postSnapshot } from '@percy/agent'

The problem with that is the util code would import other libraries, such as Winston, which are entirely designed to run on Node and not in a browser.

This PR simply removes the node-specific code from our default export, but as a result we've had to update our SDKs to reach inside @percy/agent and require these utils instead.

Those PRs are here:

Related issue:

@djones djones requested a review from Robdel12 April 6, 2019 20:48
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very very excited about this! 🏁🏁

@djones djones merged commit 4a61b78 into master Apr 8, 2019
@djones djones deleted the david-jones/avoid-including-node-code-in-default-export branch April 8, 2019 14:16
djones pushed a commit that referenced this pull request Apr 8, 2019
## [0.2.2](v0.2.1...v0.2.2) (2019-04-08)

### Bug Fixes

* do not include node-based sdk-utils in default export for package. ([#125](#125)) ([4a61b78](4a61b78))
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