Skip to content
This repository has been archived by the owner on Feb 24, 2023. It is now read-only.

Fix build error #18

Closed
wants to merge 1 commit into from
Closed

Fix build error #18

wants to merge 1 commit into from

Conversation

pedrouid
Copy link

parseNodeVersion is called on browser environment where process is undefined

@coveralls
Copy link

Pull Request Test Coverage Report for Build 63

  • 1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.5%) to 96.199%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/util.ts 1 2 50.0%
Totals Coverage Status
Change from base Build 62: -0.5%
Covered Lines: 259
Relevant Lines: 266

💛 - Coveralls

@aricart
Copy link
Member

aricart commented Dec 20, 2019

how are you running this on the browser - how big is the library - I have started porting to only use TypedArrays to remove node dependencies, but shelved the project.

@pedrouid
Copy link
Author

pedrouid commented Dec 23, 2019

I'm unsure about what's causing this to be ran on the browser.

Our SDK depends on this library and in most environments it's transpilled correctly to the browser without triggering the parseNodeVersion method. Have you ever seen this reported for Vue apps?

Also is the TypedArrays branch still recoverable on Github? Perhaps that could be a cleaner solution unless you have some concerns about it.

@aricart
Copy link
Member

aricart commented Dec 26, 2019

I am puzzled why node code is being used in any browser as a dependency, we haven't yet released our browser supported environment. Since the lib is not supported on a browser, I wouldn't expect Vue apps to be using it.

The initial release for a browser doesn't require nkeys, so browser support will be forthcoming but not yet.

@aricart aricart closed this Dec 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants