alexandercerutti / passkit-generator

The easiest way to generate custom Apple Wallet passes in Node.js
MIT License
897 stars 109 forks source link

Wrong type for wifiAccess? #135

Closed buren closed 1 year ago

buren commented 1 year ago

Running Node Version

v16.7.0

Description

Should be able to pass an array, [{ ssid: "...", password: "..." }], but I'm not allowed.

Current "working" type for passkit-generator

Screen Shot 2023-03-23 at 15 37 47

What the type should be

At least as I can tell this should work, see documentation screenshot below

Screen Shot 2023-03-23 at 15 37 37

Apple documentation

🔗 Apple docs for semantic tags

Screen Shot 2023-03-24 at 14 57 52

Expected behavior

Should be allowed to pass an array.

Steps to reproduce

Current "working" type for passkit-generator

Screen Shot 2023-03-23 at 15 37 47

What the type should be

At least as I can tell this should work, see documentation screenshot below

Screen Shot 2023-03-23 at 15 37 37

Other details

🔗 Apple docs for semantic tags 🔗 Wifi Network semantic tag

Love the project ⭐ Thanks a lot! 👑

alexandercerutti commented 1 year ago

Hey there @buren, thank you for using passkit-generator!

Thank you for your report. I guess you are right!

Are you willing to open a PR to fix this and the Joi schema? I'm unable to perform any changes right now, so if you want to open it, I will merge it directly and it will be on npm by tomorrow.

Let me know!

buren commented 1 year ago

Thank you for the quickly reply. I'm a bit confused (haven't use Joi before so bare with me 😅). It seems like wifiAccess type is an array in one place but not the other.

Object type https://github.com/alexandercerutti/passkit-generator/blob/267d8a6e8c52337f94b7ba75359f8d3cb10a26ae/src/schemas/Semantics.ts#L179

Array type https://github.com/alexandercerutti/passkit-generator/blob/267d8a6e8c52337f94b7ba75359f8d3cb10a26ae/src/schemas/Semantics.ts#L265


The below diff seems to be working for me locally. Can of course open a PR, but wanted to understand the above first (object vs array in different places). Also I had a failed test running the test suite locally with no changes to the code.

diff --git a/src/schemas/Semantics.ts b/src/schemas/Semantics.ts
index e3a8993..9a3a79b 100644
--- a/src/schemas/Semantics.ts
+++ b/src/schemas/Semantics.ts
@@ -176,7 +176,7 @@ export interface Semantics {
    venuePhoneNumber?: string;
    venueRoom?: string;

-   wifiAccess?: SemanticTagType.WifiNetwork;
+   wifiAccess?: SemanticTagType.WifiNetwork[];
 }

 export const Semantics = Joi.object<Semantics>().keys({
alexandercerutti commented 1 year ago

I gave a quick look to Joi scheme and I saw it wrong in the rush. So, yeah, it seems to be just an issue with TS.

The patch looks good to me.

but wanted to understand the above first (object vs array in different places)

I just forgot a couple of squared brackets :D

Also I had a failed test running the test suite locally with no changes to the code.

Don't worry if you just changed the types. This packages tests should get completely reworked. They where absolutely my first attempt into doing some unit tests, so they suffer of a series of problems due to my inexperience of the past.

Just to understand, which is the test that is failing?

buren commented 1 year ago

Failed test

passkit-generator git:(wifi-access-array-type) npm test                                                                                                                   git:(wifi-access-array-type|)

> passkit-generator@3.1.7 test
> npm run build:spec && npx jasmine

> passkit-generator@3.1.7 build:spec
> rimraf "./spec/*.!(ts)" && npx tsc -p tsconfig.spec.json

spec/FieldsArray.ts:13:8 - error TS2555: Expected at least 3 arguments, but got 2.

 13   fa = new FieldsArray(
           ~~~~~~~~~~~~~~~~
 14    {
    ~~~~
...
 19    pool,
    ~~~~~~~~
 20   );
    ~~~

  lib/FieldsArray.d.ts:12:83
    12     constructor(passInstance: InstanceType<typeof PKPass>, keysPool: Set<string>, fieldSchema: typeof Schemas.Field | typeof Schemas.FieldWithRow, ...args: Schemas.Field[]);
                                                                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    An argument for 'fieldSchema' was not provided.

Found 1 error in spec/FieldsArray.ts:13
alexandercerutti commented 1 year ago

Ugh, I hate them. I have to put myself into them and really create them again. Just don't worry right now.

alexandercerutti commented 1 year ago

Released in v3.1.8 🎉 Thank you for your contribution!