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: remove node.js Buffer for better browser compatibility #656

Merged
merged 3 commits into from
Dec 11, 2022

Conversation

luizstacio
Copy link
Member

@luizstacio luizstacio commented Dec 10, 2022

When using on browser env's, Buffer is not available, requiring extra bundler configs with polyfills. Using stringFromBuffer from the Keystore package, it's possible to use Buffer on node env's and TextDecoder on browsers. Fixing the issue.

For now, we don't have a CI for automating the tests on browser #284. Because of this, I have tested it locally this are the following steps;

  1. On this branch
  2. Inside packages/keystore run pnpm build and pnpm link --global --dir .
  3. Create an react app using npx create-react-app my-test-app
  4. Inside the app my-test-app run pnpm link --global @fuel-ts/keystore
  5. On the src/App.js file, paste the following code right after import './App.css';
import { decrypt, encrypt } from '@fuel-ts/keystore/dist';

const data = {
  'foo': 'bar'
};

encrypt('password', JSON.stringify(data)).then((encrypted) => {
  decrypt('password', encrypted).then((decrypted) => {
    console.log(JSON.parse(decrypted));
  });
});
  1. The console.log should show the correct data, after encrypting and decrypting.

Notes: the /dist at @fuel-ts/keystore/dist is only needed because we are using pnpm link

@github-actions
Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
91.07% (-0.06% 🔻)
3834/4210
🟡 Branches
74.07% (-0.5% 🔻)
740/999
🟢 Functions
88.01% (-0.09% 🔻)
749/851
🟢 Lines
91.1% (-0.07% 🔻)
3676/4035
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / keystore.ts
100% 50%
66.67% (-8.33% 🔻)
100%
🔴
... / aes-ctr-web.ts
23.08% (-0.92% 🔻)
100% 0%
23.08% (-0.92% 🔻)
🟡
... / script.ts
79.31% (-3.45% 🔻)
50% (-12.5% 🔻)
83.33%
79.31% (-3.45% 🔻)
🟢
... / errors.ts
100%
50% (-33.33% 🔻)
100% 100%
🟢 script/src/utils.ts
88.89% (-11.11% 🔻)
60% 100%
87.5% (-12.5% 🔻)

Test suite run success

596 tests passing in 62 suites.

Report generated by 🧪jest coverage report action from 5194510

@luizstacio luizstacio enabled auto-merge (squash) December 10, 2022 06:31
@luizstacio luizstacio self-assigned this Dec 10, 2022
@luizstacio luizstacio merged commit 0da49d3 into master Dec 11, 2022
@luizstacio luizstacio deleted the ls/fix/remove-buffer branch December 11, 2022 10:09
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.

3 participants