feat(libs): Create scaffolding for passkeys library#19984
Conversation
| "$schema": "../../../node_modules/nx/schemas/project-schema.json", | ||
| "sourceRoot": "libs/accounts/passkey/src", | ||
| "projectType": "library", | ||
| "tags": ["scope:shared:lib"], |
There was a problem hiding this comment.
👍 glad this tag is in here, it's often overlooked.
nshirley
left a comment
nshirley
left a comment
There was a problem hiding this comment.
Just a few comments, I wouldn't say any of them are blocking but curious your thoughts on my comment in the repository layer!
| * @property {Error} [cause] - The underlying error if provided | ||
| * @property {object} info - The structured info object with errno and context | ||
| */ | ||
| constructor(message: string, info: Record<string, any>, cause?: Error) { |
There was a problem hiding this comment.
I ran into this when adding tests, but it would be nice to define info as a a union like:
| constructor(message: string, info: Record<string, any>, cause?: Error) { | |
| constructor(message: string, info: { errno: number, uid: string, credentialId: string } & Record<string, any>, cause?: Error) { |
That way, we can definitely access those noted params in the doc comment, info.errno and not have to use square braces info['errno']
Just makes the other classes that extend this, and tests, a bit neater
There was a problem hiding this comment.
info: Record<string, any> is aligned with the format used in the recovery phone lib - considering that the shape of info may vary (with uid, credentialId not always being included for example), I wonder if the looser type may still be preferable? I could update the comment to mark uid, credentialId as examples vs defined arguments.
There was a problem hiding this comment.
That makes sense. I'm good leaving it as is and then I can include the change in my PR for further discussion!
Because: * We are starting passkeys implementation, this sets the base foundation This commit: * Creates libs/accounts/passkey with layered architecture (Service/Manager/Repository) * Adds PasskeyConfig class with WebAuthn-specific configuration properties * Adds PasskeyError base class with proper JSDoc for errno pattern * Creates placeholder files (repository, types) for database implementation * Documents architecture, WebAuthn concepts, and development workflow in README * Sets up unit and integration test infrastructure Closes #FXA-12900
nshirley
left a comment
nshirley
left a comment
There was a problem hiding this comment.
Looks like a really solid foundation for further work! I see some tests failed but I don't see how your changes would have done that - it looks like circle has an outage at the moment.
3 participants
Because
This pull request
Issue that this pull request solves
Closes: FXA-12900
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.