Skip to content

Commit

Permalink
fix: instanceof Uint8Array does not work correctly for vitest' en…
Browse files Browse the repository at this point in the history
…v `jsdom` (#1531)

* fix: first commit

* fix: first commit

* fix: first commit

* fix: first commit

* fix: another commit

* fix: another commit

* fix: added jsdom config for vite test

* fix: added jsdom config for vite test

* fix: added eslint rule
  • Loading branch information
freemanzMrojo authored Nov 22, 2024
1 parent 4d6a360 commit 3d4d8ce
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 26 deletions.
3 changes: 2 additions & 1 deletion apps/sdk-vite-integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"build": "tsc -b && vite build",
"lint": "eslint .",
"preview": "vite preview",
"test": "vitest"
"test": "vitest --config vitest.node.config.js",
"test:jsdom": "vitest --config vitest.jsdom.config.js"
},
"dependencies": {
"@vechain/sdk-core": "1.0.0-rc.3",
Expand Down
2 changes: 1 addition & 1 deletion apps/sdk-vite-integration/tsconfig.tsbuildinfo
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"root":["./vite.config.ts","./vitest.config.ts","./src/app.tsx","./src/main.tsx","./src/vite-env.d.ts","./src/components/getlastblock.tsx","./src/components/hash.tsx","./src/components/transferlogs.tsx","./src/const/const.tsx","./src/const/index.tsx","./src/types/index.tsx","./src/types/types.d.tsx","./tests/hash.spec.tsx"],"version":"5.6.3"}
{"root":["./vite.config.ts","./vitest.jsdom.config.ts","./vitest.node.config.ts","./src/app.tsx","./src/main.tsx","./src/vite-env.d.ts","./src/components/getlastblock.tsx","./src/components/hash.tsx","./src/components/transferlogs.tsx","./src/const/const.tsx","./src/const/index.tsx","./src/types/index.tsx","./src/types/types.d.tsx","./tests/hash.spec.tsx"],"version":"5.6.3"}
14 changes: 14 additions & 0 deletions apps/sdk-vite-integration/vitest.jsdom.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { defineConfig, ViteUserConfig } from 'vitest/config'
import react from '@vitejs/plugin-react'

export default defineConfig({
plugins: [react()] as ViteUserConfig["plugins"],
test: {
environment: 'jsdom',
browser: {
enabled: true,
name: 'chromium',
provider: 'playwright',
},
},
})
File renamed without changes.
32 changes: 32 additions & 0 deletions eslint-local-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,37 @@ module.exports = {
}
};
}
},
'disallow-instanceof-uint8array': {
meta: {
type: 'problem', // Define the rule type
docs: {
description: 'Disallow usage of instanceof Uint8Array',
category: 'Best Practices',
recommended: false
},
schema: [], // No options for this rule
messages: {
avoidInstanceOfUint8Array:
'Please review if you can avoid using instanceof Uint8Array. If not, please use ArrayBuffer.isView instead.'
}
},
create(context) {
return {
BinaryExpression(node) {
// Check if it's an instanceof expression
if (
node.operator === 'instanceof' &&
node.right.type === 'Identifier' &&
node.right.name === 'Uint8Array'
) {
context.report({
node,
messageId: 'avoidInstanceOfUint8Array'
});
}
}
};
}
}
};
1 change: 1 addition & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export default [{
"no-warning-comments": "warn",
"no-multi-str": "off",
"local-rules/disallow-buffer-from-alloc": "error",
"local-rules/disallow-instanceof-uint8array": "error",
"sonarjs/different-types-comparison": "off",
"sonarjs/no-ignored-exceptions": "off",
"sonarjs/no-nested-functions": "off",
Expand Down
19 changes: 10 additions & 9 deletions packages/core/src/vcdm/Hex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,7 @@ class Hex implements VeChainDataModel<Hex> {
*/
public static of(exp: bigint | number | string | Uint8Array): Hex {
try {
if (exp instanceof Uint8Array) {
return new Hex(this.POSITIVE, nc_utils.bytesToHex(exp));
} else if (typeof exp === 'bigint') {
if (typeof exp === 'bigint') {
if (exp < 0n) {
return new Hex(
this.NEGATIVE,
Expand All @@ -287,8 +285,14 @@ class Hex implements VeChainDataModel<Hex> {
exp < 0 ? this.NEGATIVE : this.POSITIVE,
nc_utils.bytesToHex(new Uint8Array(dataView.buffer))
);
}
if (this.isValid(exp)) {
} else if (typeof exp === 'string') {
if (!this.isValid(exp)) {
throw new InvalidDataType(
'Hex.of',
'not an hexadecimal string',
{ exp }
);
}
if (exp.startsWith('-')) {
return new Hex(
this.NEGATIVE,
Expand All @@ -302,10 +306,7 @@ class Hex implements VeChainDataModel<Hex> {
this.REGEX_HEX_PREFIX.test(exp) ? exp.slice(2) : exp
);
}
// noinspection ExceptionCaughtLocallyJS
throw new InvalidDataType('Hex.of', 'not an hexadecimal string', {
exp
});
return new Hex(this.POSITIVE, nc_utils.bytesToHex(exp));
} catch (e) {
throw new InvalidDataType(
'Hex.of',
Expand Down
14 changes: 9 additions & 5 deletions packages/core/src/vcdm/Revision.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Txt } from './Txt';
import { Hex } from './Hex';
import { InvalidDataType } from '@vechain/sdk-errors';
import { Hex } from './Hex';
import { HexUInt } from './HexUInt';
import { Txt } from './Txt';

/**
* Represents a revision for a Thor transaction or block.
Expand Down Expand Up @@ -62,10 +62,14 @@ class Revision extends Txt {
let txt: string;
if (value instanceof Hex) {
txt = value.bi.toString();
} else if (value instanceof Uint8Array) {
txt = Txt.of(value).toString();
} else {
} else if (
typeof value === 'bigint' ||
typeof value === 'number' ||
typeof value === 'string'
) {
txt = `${value}`;
} else {
txt = Txt.of(value).toString();
}
if (Revision.isValid(txt)) {
return new Revision(txt);
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/vcdm/Txt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ class Txt extends String implements VeChainDataModel<Txt> {
* @returns {Txt} - A new Txt instance.
*/
public static of(exp: bigint | number | string | Uint8Array): Txt {
if (exp instanceof Uint8Array) {
return new Txt(Txt.DECODER.decode(exp));
if (typeof exp === 'string') {
return new Txt(exp);
} else if (typeof exp === 'bigint' || typeof exp === 'number') {
return new Txt(exp.toString());
}
return new Txt(exp);
return new Txt(Txt.DECODER.decode(exp));
}
}

Expand Down
14 changes: 9 additions & 5 deletions packages/core/src/vcdm/encoding/rlp/RLP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@ class RLP implements VeChainDataModel<RLP> {
protected constructor(data: RLPInput);
protected constructor(data: Uint8Array);
protected constructor(data: RLPInput | Uint8Array) {
this.decoded =
data instanceof Uint8Array ? EthereumjsRLP.decode(data) : data;
this.encoded =
data instanceof Uint8Array ? data : EthereumjsRLP.encode(data);
// ArrayBuffer.isView so we support https://github.com/vitest-dev/vitest/issues/5183
this.decoded = ArrayBuffer.isView(data)
? EthereumjsRLP.decode(data)
: data;
this.encoded = ArrayBuffer.isView(data)
? data
: EthereumjsRLP.encode(data);
}

/**
Expand Down Expand Up @@ -214,7 +217,8 @@ class RLP implements VeChainDataModel<RLP> {

// ScalarKind: Direct decoding using the provided method.
if (kind instanceof ScalarKind) {
if (!(packed instanceof Uint8Array)) {
// ArrayBuffer.isView so we support https://github.com/vitest-dev/vitest/issues/5183
if (!ArrayBuffer.isView(packed)) {
throw new InvalidRLP(
'RLP.unpackData()',
`Unpacking error: Expected data type is Uint8Array.`,
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/vcdm/encoding/rlp/kind/BufferKind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class BufferKind extends ScalarKind {
*/
public data(data: RLPInput, context: string): DataOutput {
// Ensure that the data is indeed a Buffer before encoding.
if (!(data instanceof Uint8Array))
// ArrayBuffer.isView so we support https://github.com/vitest-dev/vitest/issues/5183
if (!ArrayBuffer.isView(data)) {
throw new InvalidRLP(
'BufferKind.data()',
`Validation error: Expected a Uint8Array type in ${context}.`,
Expand All @@ -28,6 +29,7 @@ class BufferKind extends ScalarKind {
}
}
);
}

return {
encode: () => data // Data is already a Buffer, so return as-is.
Expand Down
7 changes: 6 additions & 1 deletion packages/core/tests/vcdm/Txt.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ describe('Txt class tests', () => {
});

test('Different text encoding should result in equal bytes', () => {
expect(Txt.of(diacritic).bytes).toEqual(Txt.of(normal).bytes);
const expected = Txt.of(diacritic).bytes;
expect(expected).toEqual(Txt.of(normal).bytes);

// eslint rule test
// eslint-disable-next-line local-rules/disallow-instanceof-uint8array
expect(expected instanceof Uint8Array).toBe(true);
});
});
});

1 comment on commit 3d4d8ce

@github-actions
Copy link

Choose a reason for hiding this comment

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

Test Coverage

Summary

Lines Statements Branches Functions
Coverage: 99%
98.99% (4345/4389) 97.54% (1390/1425) 98.9% (901/911)
Title Tests Skipped Failures Errors Time
core 827 0 💤 0 ❌ 0 🔥 2m 18s ⏱️
network 719 0 💤 0 ❌ 0 🔥 4m 46s ⏱️
errors 42 0 💤 0 ❌ 0 🔥 17.3s ⏱️
logging 3 0 💤 0 ❌ 0 🔥 17.652s ⏱️
hardhat-plugin 19 0 💤 0 ❌ 0 🔥 1m 3s ⏱️
aws-kms-adapter 23 0 💤 0 ❌ 0 🔥 1m 27s ⏱️
ethers-adapter 5 0 💤 0 ❌ 0 🔥 1m 11s ⏱️
rpc-proxy 37 0 💤 0 ❌ 0 🔥 1m 3s ⏱️

Please sign in to comment.