-
Notifications
You must be signed in to change notification settings - Fork 24
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
The global state refactor #422
Conversation
7b3efbd
to
d38edfa
Compare
fd6048c
to
feb2d1b
Compare
b4765e2
to
9a19aa9
Compare
"prebuild": "rimraf lib dist && node scripts/gen-version.js", | ||
"build": "yarn run build:ipfs && tsc && yarn run build:minified", | ||
"build": "tsc && npm run build:minified", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced yarn with npm.
}) | ||
|
||
import pkg from "../package.json" assert { type: "json" }; | ||
import lock from "../package-lock.json" assert { type: "json" }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May need Node v18.
export async function bareNameFilter( | ||
{ crypto, accountDID, path }: Arguments | ||
): Promise<string> { | ||
return `wnfs:${accountDID}:bareNameFilter:${await pathHash(crypto, path)}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File systems are namespaced by account DID, allows for multiple filesystems to be loaded at the same time and temporary filesystems.
import { CID } from "multiformats/cid" | ||
|
||
|
||
export type Implementation = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preparation for CAR mirror/pool, allows us to switch out js-ipfs.
export const inMemoryDepot: Record<string, Uint8Array> = {} | ||
|
||
|
||
const depot: Depot.Implementation = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This avoids us having to use js-ipfs in the tests.
Bit hacky I guess because I shove large chunked files inside a single block, but it works.
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ | ||
%@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@% | ||
|
||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most important file, everything comes together here. The documentation should be self-explanatory, note if anything is not clear or missing.
|
||
const authedUsername = await common.authenticatedUsername() | ||
// PREDEFINED COMPONENT COMBINATIONS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this section for a detailed explanation on the components and how they work together. A short explanation is located in the changelog.
Left a bunch of comments here, some thoughts, questions and additional notes for other people to read. |
src/index.ts
Outdated
{ | ||
implementation: Auth.Implementation<Components> | ||
|
||
accountConsumer: (username: string) => Promise<AccountLinkingConsumer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to separate the functions that a user/dev directly uses from the implementation. Mixing the implementation functions in here would be confusing, for example, linkDevice
does not fully link a device, you have to create an account linking consumer & producer.
* | ||
* See `assemble` for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we somehow make it so that typedoc inserts a link here to the documentation of this assemble
function?
Co-authored-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com> Signed-off-by: Steven Vandevelde <icid.asset@gmail.com>
There's some remaining usage of "confidences" in the README and CHANGELOG ✌️ @icidasset |
Co-authored-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com> Signed-off-by: Steven Vandevelde <icid.asset@gmail.com>
Thanks for spotting that! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work! So much accomplished here. It's looking great! 💯 🎉
As a summary, here are the things I have tested manually:
webnative.compositions
withstaging
set to true- Account linking (tested in the Webnative app template on staging and production)
- Backwards compatibility (tested with staging and production)
- Custom manners component with alterations to logging
- Custom manners component with filesystem hooks (using Add file system hooks #428)
- Tested with the Stored Wasm example which uses
capabilities
and the Fission Auth Lobby
Thanks for all these changes! Excited to start using them in our apps. 🕺
* Add file system hooks * Use FileSystem interface instead of type arg * Less changes * Add Properties interface * Pass CID to beforeLoadExisting hook * Re-export CID from common/cid * Provide the data flow components as arguments * Remove unnecessary cast to Uint8Array * Pass AssociatedIdentity to hooks * Remove Exchange interface * Don't add sample data by default * DataFlowComponents -> DataComponents
Co-authored-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com> Signed-off-by: Steven Vandevelde <icid.asset@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked through quite a bit of this PR and I got a feel for the refactor when working a bit on the dashboard.
I'd say we can go ahead and :)
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com> Signed-off-by: Steven Vandevelde <icid.asset@gmail.com>
Co-authored-by: Philipp Krüger <philipp.krueger1@gmail.com> Signed-off-by: Steven Vandevelde <icid.asset@gmail.com>
Summary
This refactor gets rid of the global state throughout webnative. Things such as the
setup
object and dependency injection now work entirely different. Instead of accessing a global object, we now createProgram
instances which have pointers to various instances of components that we pass to functions. For example, we create a crypto component and that is passed to a function that does crypto things.See CHANGELOG.md for more info.
Tasks
reference
(dns + did lookup/update + data lookup/update + repos)manners
(responsible for config dependent behaviours, eg. debug loggging)program
entrypointindex.ts
)iss
and have the dev always set it?)depot
component instead. UseUint8Array
s as much as possible.Code quality checks
keystore-idb
code is only used in theCrypto
implementation code and nowhere else.SymmAlg
should only be imported fromcomponents/crypto/implementation
Depot
implementation, everything else should be Depot + DAGs /IPLD + CIDsManual testing
Will need updating
Test with alpha version before release.
Closes issues
Closes #418, #283, #274