Skip to content

Commit

Permalink
fix latitudegames#15 in another way by removing textEncoder dependency
Browse files Browse the repository at this point in the history 
By using the Buffer class in this way, you can encode and decode strings as UTF-8 without using the TextEncoder and TextDecoder classes. This can be useful if you need to support earlier versions of Node.js that do not have these classes in the util module.
  • Loading branch information
syonfox committed Dec 25, 2022
1 parent 6354a3c commit 1831e5b
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions Encoder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ const chr = x => {
return String.fromCharCode(x)
}

const textEncoder = new TextEncoder("utf-8")
const encodeStr = str => {
return Array.from(textEncoder.encode(str)).map(x => x.toString())
return Array.from( Buffer.from(str, 'utf-8')).map(x => x.toString());
}

const textDecoder = new TextDecoder("utf-8")
const decodeStr = arr => {
return textDecoder.decode(new Uint8Array(arr));
return Buffer.from(arr).toString('utf-8')
}

const dictZip = (x, y) => {
Expand Down

10 comments on commit 1831e5b

@syonfox
Copy link
Owner

Choose a reason for hiding this comment

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

seems to still pass tests in node 16 please test in 12 if that's what you use and if it works will bump npm

@mywristbands
Copy link

Choose a reason for hiding this comment

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

Thanks, this fixed my issue. When using the actual gpt-3-encoder npm library, in my environment I get the error failed to execute source for 'node_modules/gpt-3-encoder/Encoder.js': ReferenceError: 'TextEncoder' is not defined at node_modules/gpt-3-encoder/Encoder.js:33:23(86). But with this fork, it now works with no errors. I don't know exactly what version of Node.js is being used in my environment, as I am using Atlas App Services to host my application, but I assume it's an older version.

@syonfox
Copy link
Owner

@syonfox syonfox commented on 1831e5b Jan 5, 2023

Choose a reason for hiding this comment

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

Hmm Im not sure I pushed to npm yet do you know if you used this branch of the npm version?

@mywristbands
Copy link

@mywristbands mywristbands commented on 1831e5b Jan 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Right now I'm targeting the master branch of this repo, specifically at this commit. Here's the dependency I added to my package.json:

"@syonfox/gpt-3-encoder": "github:syonfox/GPT-3-Encoder#1831e5b24d3890ba8fdb00b3c937975cdef2323b"

@mywristbands
Copy link

Choose a reason for hiding this comment

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

I'm actually getting an error when I try to install this GitHub repo within my GitHub Actions CI pipeline:

npm ERR! Error while executing:
npm ERR! /usr/bin/git ls-remote -h -t ssh://[email protected]/syonfox/GPT-3-Encoder.git
npm ERR! 
npm ERR! [email protected]: Permission denied (publickey).
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.
npm ERR! 
npm ERR! exited with error code: 128

I tried changing the line in my package.json to the below, but I'm still getting the same error.

"@syonfox/gpt-3-encoder": "git+https://github.com/syonfox/GPT-3-Encoder.git",

So, I think I'll need to wait until you push this commit to npm.

@syonfox
Copy link
Owner

@syonfox syonfox commented on 1831e5b Jan 7, 2023

Choose a reason for hiding this comment

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

Ok Still passes tests published 1.3.0 for this

@syonfox
Copy link
Owner

@syonfox syonfox commented on 1831e5b Jan 7, 2023

Choose a reason for hiding this comment

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

https://github.com/syonfox/GPT-3-Encoder
Ok, i updated the read me as well if you feel inclined let me know how you set up the working pipeline that seems useful I have we could maybe add an example to the usage docs.

@mywristbands
Copy link

@mywristbands mywristbands commented on 1831e5b Jan 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, it was very nice of you to push the changes so quickly. I added your package directly from the npm registry, and it is now installing without errors in my GitHub Actions CI environment!

I'm happy to share how I set up the pipeline, but I'm not sure if any of it would be relevant to your usage docs. The actual installation of my project's node packages is a pretty simple part of my pipeline. I just run npm ci, and it installs all the node packages into my CI environment. I do happen to be using node version 14 in the environment, if that's useful to know.

@syonfox
Copy link
Owner

@syonfox syonfox commented on 1831e5b Jan 8, 2023

Choose a reason for hiding this comment

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

Cool, the GitHub tests are set to node 12 and node 16 so if they pass and yours works I say we are golden. I spent the last little bit touching up some stuff and adding browserify support so we can use a single 2.5mb bundle in the browser to do this on the client. If you feel inclined let me know if the 1.4rc2 works there is a change in how I load the blob of characters.

@mywristbands
Copy link

Choose a reason for hiding this comment

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

Sure, I just updated to 1.4rc3, and everything's working correctly for me.

Please sign in to comment.