Skip to content

Added keyed/aberdeen framework #1877

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

vanviegen
Copy link

Added a benchmark for https://aberdeenjs.org/

Is this (just including the prebuilt js file for the lib, like keyed/alpine does) an okay way to do it?

Thanks for taking the time!

@krausest
Copy link
Owner

aberdeen.js is currently not included in the PR.
I'd really prefer to use npm to install the libs. This allows me to update implementations and to track how old the libs are.
Can you update the PR to install via npm?

@vanviegen
Copy link
Author

@krausest, oops! The .gitignore for dist got me.

I've now added the library as an npm dependency, and added a build step.

@krausest
Copy link
Owner

Thanks - it builds now fine, but my check (npm run isKeyed keyed/aberdeen)complains it's not really keyed:

Keyed test for swap failed. Swap must add the TRs that it removed, but there were 2 new nodes
Keyed test for create rows failed. Expected that 1000 TRs should be removed and added, but there were 0 added TRs and 0 were removed
aberdeen-v1.0.4-keyed is non-keyed for 'run benchmark' and keyed for 'remove row benchmark' and non-keyed for 'swap rows benchmark' . It'll appear as non-keyed in the results
ERROR: Framework aberdeen-v1.0.4-keyed is not correctly categorized

Can you take a look at it please?

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.

2 participants