Skip to content

allow components to be locked to block height#9

Closed
mpeterdev wants to merge 2 commits intoNearSocial:masterfrom
mpeterdev:feat/lock-component-version
Closed

allow components to be locked to block height#9
mpeterdev wants to merge 2 commits intoNearSocial:masterfrom
mpeterdev:feat/lock-component-version

Conversation

@mpeterdev
Copy link
Contributor

@mpeterdev mpeterdev commented Mar 7, 2023

I branched off 0.3.0 for testing in alpha.near.org — will rebase against main branch once changes are conceptually approved

note that this is only an attempt at block height versioning for meeting basic needs. We should trend towards semver as a best practice but that will take more time to implement

example usage: <Widget src="mpmain2.near/widget/VersionTestInner" version="86775507" />
example usage: <Widget src="mpmain2.near/widget/VersionTestInner@86775507" />

Copy link
Contributor

@evgenykuzyakov evgenykuzyakov left a comment

Choose a reason for hiding this comment

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

  1. I'd prefer if we lock version using @ sign as part of src. For example a.near/widget/Widget@524534. This way it makes it easier to lock it within a direct url. We can also use branches the same way
  2. We should think about the default logic for children components. My original idea for block versions is that all children components will be automatically locked based on the height of the parent component. But it also might be undesirable in some situations. So we can add some modifiers on top of the height. E.g.
  • @=524534
  • @^524534
  • or something like this.

return acc[curr];
}, componentVersionRes);
// console.log("component versions", componentVersions);
if (!componentVersions.includes(version)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is quite necessary to check history, and this is also an expensive call. Probably the API can return a blockHeight at which the data was stored. So we can compare it this way if it's required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, I was leaning towards throwing an error if a matching version wasn't found to catch user error of specifying an incorrect block, but it doesn't need to be done here. If we care about that, we could do it during editing

@mpeterdev
Copy link
Contributor Author

mpeterdev commented Mar 7, 2023

  1. I'd prefer if we lock version using @ sign as part of src. For example a.near/widget/Widget@524534. This way it makes it easier to lock it within a direct url. We can also use branches the same way

I equally considered going that way, happy to make the change

  1. We should think about the default logic for children components. My original idea for block versions is that all children components will be automatically locked based on the height of the parent component. But it also might be undesirable in some situations. So we can add some modifiers on top of the height. E.g.
  • @=524534
  • @^524534
  • or something like this.

Auto-locking is interesting. I would be inclined to push this change through such that it enables manual locking but doesn't affect global default behavior, then spin up a thread with more stakeholders to discuss changing default behavior as a later add-on.

For the example of @^524534, wouldn't that always just translate to latest without semver to define a max?

How about the following:

@<block>
e.g. @524534

@v<semver>
e.g. @v^5.0.0

alternatively for semver, do it npm-like

@semver:<semver>
e.g. @semver:^5.0.0

down the road, allow explicitly specifying no lock

@latest

if we decide to always require a version and want to support locking to parent, I could think of a number of options

@here @this @publish @parent

@mpeterdev mpeterdev closed this Mar 7, 2023
@mpeterdev mpeterdev reopened this Mar 7, 2023
@evgenykuzyakov
Copy link
Contributor

I think this is a good start

@mpeterdev
Copy link
Contributor Author

closed in favor of #10

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