Skip to content
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

Dependency updates #652

Merged
merged 11 commits into from
Mar 12, 2024
Merged

Dependency updates #652

merged 11 commits into from
Mar 12, 2024

Conversation

farskipper
Copy link
Member

@farskipper farskipper commented Nov 12, 2023

  • The pico-engine packages have all been updated to run on the latest Node.js
  • Only breaking change effects direct consumers of pico-framework or pico-engine-core. Described here
  • Updated the github actions to ensure tests will run against the LTS and Current node versions on Linux/Mac/Windows.
  • Most of the npm packages have been updated. There are some exceptions outlined below.

What changed by package

  • select-when
    • No changes to the user
    • Updated typescript, setup testing
    • Published 0.1.9
  • pico-framework
    • Only breaking change to how the database is configured. Described here
      • This only effects those who consume pico-framework directly
    • Updated dependencies minus cuid (see below)
    • Setup testing
    • Published 0.7.0
  • krl-compiler
    • No changes to the user
    • Updated dependencies minus standard (see below)
  • krl-generator
    • No changes to the user
    • Updated dependencies minus standard (see below)
  • krl-parser
    • No changes to the user
    • Updated dependencies
  • krl-stdlib
    • No changes to the user
    • Updated dependencies
  • pico-engine-core
    • Breaking change in how the db is configured. Same change as pico-framework.
      • You pass in a leveldb that meets the PicoDb api rather than a leveldown instance.
      • This only effects those who consume pico-core directly
    • Updated dependencies minus cuid,bs58,normalize-url,p-memoize,random-words (see below)
  • pico-engine-ui
    • Switched from Webpack to Vite
      • It's a newer alternative that is much simpler to configure, and faster
      • Switching to this was easier than update all the webpack depencies
      • No change to development workflow.
    • Updated dependencies including React 18
      • NOTE: Moved bootstrap from npm to a simple .min.css file
        • The new scss compiler doesn't like bootstrap v4's scss code.
        • Since we don't customize bootstrap there's no need to leave it in npm.
        • Upgrading to v5 does not seem worth it.
  • pico-engine
    • No changes to the user.
    • Updated dependencies minus cuid and rotating-file-stream (see below)

Dependencies not updated

Package               Current  Wanted  Latest  Location                           Depended by
lerna                  3.22.1  3.22.1   8.0.0  node_modules/lerna                 root
node-gyp                9.4.1   9.4.1  10.0.1  node_modules/node-gyp              root
standard               12.0.1  12.0.1  17.1.0  node_modules/standard              krl-compiler
standard               12.0.1  12.0.1  17.1.0  node_modules/standard              krl-generator
cuid                    2.1.8   2.1.8   3.0.0  node_modules/cuid                  pico-engine
rotating-file-stream    1.4.6   1.4.6   3.1.1  node_modules/rotating-file-stream  pico-engine
bs58                    4.0.1   4.0.1   5.0.0  node_modules/bs58                  pico-engine-core
cuid                    2.1.8   2.1.8   3.0.0  node_modules/cuid                  pico-engine-core
normalize-url           5.3.1   5.3.1   8.0.0  node_modules/normalize-url         pico-engine-core
p-memoize               4.0.4   4.0.4   7.1.1  node_modules/p-memoize             pico-engine-core
random-words            1.3.0   1.3.0   2.0.0  node_modules/random-words          pico-engine-core
  • lerna
    • The project has significant changes, and not wroth updating at this time
    • This doesn't effect consumers of the pico-engine
    • npm now has workspaces, so it's possible we could migrate off of lerna
  • node-gyp
    • I left this as is since I don't know why it's needed on the root package.json
  • standard
    • This is the linter/formatter used on krl-compiler and krl-generator. Only used for development.
    • Upgrading this would require re-formatting all the files. I don't think it's worth it at this time since it'd be so noisy.
    • Ideally we'd move away from standard and use prettier like the other packages.
  • cuid
    • We use this to create unique ids for channels among other things.
    • cuid's include a timestamp and machine thumbprint
    • The maintainer deprecated cuid since the id's can be guessed, therefore insecure
    • Changing this could be significant b/c I'm not sure if people depend on the structure of the ids to include timestamps and sortability.
  • rotating-file-stream
    • I'm not as familiar with how pico-engine logs are used in the real world.
    • The package has good documentation on what was changed for v2 and v3
    • Leaving it as is works fine on the latest Nodejs
  • bs58
    • They changed it to use Uint8Array
    • I'm not as familiar with the ursa and dido krl modules, so I opted to leave it alone.
  • normalize-url, p-memoize, random-words
    • All of these require moving to the new ESModule standard
    • The annoying thing is this would require converting our packages to ESM which changes how users consume it.

@farskipper farskipper requested a review from b1conrad November 12, 2023 05:55
@farskipper farskipper self-assigned this Nov 12, 2023
Copy link
Member

@b1conrad b1conrad left a comment

Choose a reason for hiding this comment

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

I have a rough idea of how you are proceeding, and don't see anything obviously amiss. Thank you @farskipper for doing this.

@farskipper farskipper marked this pull request as ready for review November 26, 2023 00:39
@farskipper farskipper changed the title [WIP] Dependency updates Dependency updates Nov 26, 2023
@b1conrad
Copy link
Member

b1conrad commented Jan 4, 2024

A note on cuid in response to your not knowing "if people depend on the structure of the ids". We do not depend on the internal structure of an ECI. It is used only as an opaque identifier to locate the corresponding pico and policy.

We do sort the list of channels by the identifier, but this is solely to expedite finding a channel in the list when we know its identifier and are looking for it for some reason (usually to see its policy).

As a result, there should be no adverse consequence of moving to cuid2. Pre-existing channels (created before an upgrade) would not have their identifiers changed, but channels created after an update would have new identifiers. The fact that channel identifiers currently sort in the order they were created would no longer be a fact, and that might impact certain pico developer practices. I sometimes use that fact, but could easily work around the change. It might be helpful to include a time value in each channel so that they could be sorted by it, but that could be an enhancement request after an upgrade.

Bottom line is that cuid could be upgraded to cuid2 for security purposes without a disruption big enough to be classified as "breaking".

@farskipper
Copy link
Member Author

farskipper commented Jan 6, 2024

@b1conrad Ok, good to know the cuid structure is not a dependency.

Another alternative is nodejs's built-in UUID implementation.

To store the channel creation time, it can be added to pico-framework
While in there, it'd be good to also store the last updated time, and do the same on the Pico object.

@b1conrad
Copy link
Member

b1conrad commented Mar 8, 2024

Put a copy of an existing db folder in place, and started with

PICO_ENGINE_HOME=. PORT=3002 node dist/cli.js

and got a spate of strange error messages like

Error: Ruleset io.picolabs.subscription does not have query function "established"
    at Pico.doTxn (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/Pico.js:395:31)
    at PicoQueue.doWork (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/PicoQueue.js:47:35)
    at Timeout._onTimeout (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/PicoQueue.js:22:31)
    at listOnTimeout (node:internal/timers:573:17)
    at process.processTimers (node:internal/timers:514:7)

in the log, presumably for the established subscriptions among the picos. Tried the Testing tab and got a similar message about there not being a query function for __testing.

So, assumption is that that ruleset needs to be recompiled (and that doesn't trigger automatically because both engines are still at the same version until we merge and publish a new version number).

Also got one occurrence of this error

TypeError: ruleset.ruleset.init is not a function
    at CorePico.use (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/dist/CorePico.js:28:43)
    at Object.useModule (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/dist/makeKrlCtx.js:54:28)
    at Object.init (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine/rulesets/1.3.0/4a/03/4a03e1844c573fb98990f2e884965631fe4ae5f8b7bb1451310c9c937816efab.js:251:16)
    at Pico.installBase (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/Pico.js:259:35)
    at Pico.install (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/Pico.js:238:48)
    at Object.install (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/RulesetContext.js:45:25)
    at Object.install (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/dist/modules/ctx.js:120:30)
    at async Object.body (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine/rulesets/1.3.0/1b/df/1bdf14cdcdc62c15ed75c33c391b77ca82097e218f16b12c6b55bdf690d67e3e.js:759:22)

and stopped the engine. (Looking, and that happens to be io.picolabs.wrangler FWIW.)

Removed rulesets and rulesets-db and restarted engine. Everything seemed to work. Removed db and the rulesets* folders and recopied the db into place to start again from scratch. It stopped with this

Failed to start engine.
TypeError: rs.init is not a function
    at Pico.installBase (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/Pico.js:259:35)
    at Pico.install (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine-core/node_modules/pico-framework/dist/src/Pico.js:238:48)
    at startEngine (/Users/bruceconrad/Documents/b1conrad/dependency-updates/pico-engine/packages/pico-engine/dist/index.js:54:27)

But I just ran it again, and it seemed to work. Removed the rulesets* folders and started it with the same failure. Trying again... Seems to be working. Looking at some picos... Seems to be running.

@b1conrad b1conrad merged commit fd8f80d into master Mar 12, 2024
12 checks passed
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