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

add full Semantic Versioning for Shared Modules #11105

Merged
merged 11 commits into from Jul 6, 2020
Merged

add full Semantic Versioning for Shared Modules #11105

merged 11 commits into from Jul 6, 2020

Conversation

sokra
Copy link
Member

@sokra sokra commented Jun 30, 2020

  • change storage format in the share scope
    • We have to lift the limitation of having only one version per version position, as complex ranges could select lower versions
    • Instead we walk through all version available while consuming to find the highest in range
    • In the share scope we now store it this way: shareScope[shareKey][version] = { get, loaded, from }
      • from is now the uniqueName. When providing to the shareScope implementations should now override a version when from is equal or higher.
  • choose shared module deterministic from the same origin
    • output.uniqueName is used as second criteria if multiple builds provide the same version
    • This improves long term caching as used shared module do no longer depend on a race condition
  • remove array syntax from version and requiredVersion in schema
    • The new format is internal and should not be exposed to the user or API
    • And we want to have the option to change this later

Version are encoding this way:

1.2.3 => [1,2,3]
1.2.3-beta.42 => [1,2,3,,"beta",42]
1.2.3.4.5+55.66 => [1,2,3,4,5,[],55,66]
1.alternative-beta.42+55.66 => [1,"alternative",,"beta",42,[],55,66]

This format allows easy walking and decision based on typeof item. The duplicate ,, is intentional and creates a hole, which is typeof undefined, but very short to print^^

The format is SemVer compatible but less restrictive. We allow more and less version numbers. We allow strings as version numbers (might be useful for a/b tests). We do not apply additional restrictions on the possible chars.

Ranges are encoding this way:

Simple ranges
1.2.3, v1.2.3, =1.2.3 => [4,1,2,3] (4 fixed positions, 1,2,3 is the version
>=1.2.3 => [0,1,2,3]
^1.2.3 => [1,1,2,3]
^0.2.3 => [2,0,2,3]
^0.0.3 => [3,0,0,3]
<1.2.3 => [-1,1,2,3] (negative changes direction)
5, 5.x, 5.*, 5.X, 5.x.x, ... => [1,5]
9.8, 9.8.*, ... => [2,9,8]

Complex ranges
(They compile to a little program, executing simple ranges and operations on a stack)
(Complex ranges are identified by starting with a hole (!(0 in range)))
^4 || ^5 => [,[1,5],[1,4],1] (1 is the operation for OR)
>=2 <3 => [,[-1,3],[0,2],2] (2 is the operation for AND)
>1.2.3 => (>=1.2.3 not(1.2.3)) => [,[4,1,2,3],0,[0,1,2,3],2] (0 is the operation of NOT)
<=1.2.3 => [,[4,1,2,3],[-1,1,2,3],1]
1.2.3 - 3.2.1 => (>=1.2.3 <=3.2.1) => [,[4,3,2,1],[-1,3,2,1],1,[0,1,2,3],2]

The version and range encoding is only used webpack internal.

Sadly all this additional logic increases the bundle size by about 750b. The most logic is on the consuming shared modules side. But as the needed logic depends on the consumed shared version ranges in the application, there is improvement possible in future PRs. If only simple ranges (without prerelease/build tag) are used we could use a reduced satisfy version. In production we could omit rangeToString and use JSON.stringify instead. This would decrease the quality of errors/warnings, but might make sense for production.

What kind of change does this PR introduce?
feature

Did you add tests for your changes?
yes

Does this PR introduce a breaking change?
yes (containers are not compatible to previous beta versions)

What needs to be documented once your changes are merged?
The semver limitation can be removed.

@webpack-bot
Copy link
Contributor

webpack-bot commented Jun 30, 2020

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@jasongrout
Copy link
Contributor

1.2.3 - 3.2.1 => (>=1.2.3 <=3.2.1) => [,[4,3,2,1],[-1,3,2,1],1,[0,1,2,3],1]

This should encode as [,[4,3,2,1],[-1,3,2,1],1,[0,1,2,3],2] (last operation is an AND), right?

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

Copy link
Contributor

@jasongrout jasongrout left a comment

Choose a reason for hiding this comment

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

A few notes from looking through things so far.

lib/sharing/utils.js Outdated Show resolved Hide resolved
lib/sharing/utils.js Outdated Show resolved Hide resolved
test/SemVer.unittest.js Outdated Show resolved Hide resolved
test/SemVer.unittest.js Show resolved Hide resolved
@webpack-bot
Copy link
Contributor

The minimum test ratio has been reached. Thanks!

lib/util/semver.js Outdated Show resolved Hide resolved
lib/util/semver.js Outdated Show resolved Hide resolved
lib/util/semver.js Outdated Show resolved Hide resolved
test/SemVer.unittest.js Outdated Show resolved Hide resolved
test/SemVer.unittest.js Outdated Show resolved Hide resolved
@vankop
Copy link
Member

vankop commented Jul 2, 2020

another interesting trick for you =)
As I see there is a lot of checks a = (typeof element)[0] a separator (a == "u" || a == "o") or vise versa (a == "n" || a == "s"),
but if you take second char you get useful sequence
(typeof []).charCodeAt(1) === 98
(typeof undefined).charCodeAt(1) === 110
(typeof "s").charCodeAt(1) === 116
(typeof 1).charCodeAt(1) === 117
so you get "number" > "string" > "undefined" > "object"
Simplified checks a < "t" //separator, a > "n" // value

Probably there is further improvement with & 24 to get "number" === "string" > "undefined" > "object"

const type = (typeof element).charCodeAt(1)&24;
type > 0 // undefined,string,number
type > 8 // string,number
type < 16 // undefined,array

sokra added 2 commits July 2, 2020 16:09
change storage format in the share scope
choose shared module deterministic from the same origin
remove array syntax from version and requiredVersion in schema
sokra added 7 commits July 2, 2020 16:09
versions are in a contiguous range of:
0, 1, ..., 9, 10, 11, ..., 100, ..., a, b, c, ..., alpha, ..., beta, ..., etc, ...
share scope uses string version instead of parsed one
build version should be ignored in matching ranges
@sokra
Copy link
Member Author

sokra commented Jul 2, 2020

so you get "number" > "string" > "undefined" > "object"

That's actually a useful order. I only tries this with the first char, which had number and string separated and was meh.

Char 5 is also interesting as allows to check with &2 for string/number vs undefined/object, and &1 for string/undefined vs number/object.

If you are interested to optimize this, feel free to. There are many unit tests to test this against. Currently this PR adds about 750b bundle overhead compared to the old solution.

I think I will merge it in this state, as it seems to work.

I did a little refactoring to make all the parsed version and internal representations internal. So this is no longer part of the container API and we can change it in any way.

The container API as now the version as string stored. That makes it more portable anyway.

@webpack-bot
Copy link
Contributor

I've created an issue to document this in webpack/webpack.js.org.

@jasongrout
Copy link
Contributor

The container API as now the version as string stored. That makes it more portable anyway.

I'm still seeing moduleToHandlerMapping in remoteEntry.js storing the semver range as a parsed list in webpack 5b21:

/******/ 		var installedModules = {};
/******/ 		var moduleToHandlerMapping = {
/******/ 			"webpack/sharing/consume/default/@jupyterlab/application": () => loadSingletonVersionCheck("default", "@jupyterlab/application", [1,3,0,0,,"alpha",0]),
/******/ 			"webpack/sharing/consume/default/@jupyterlab/coreutils": () => loadSingletonVersionCheck("default", "@jupyterlab/coreutils", [1,4,1,0]),

I thought all semver ranges were going to be encoded as strings in the js bundles, and the list encoding was going to be internal implementation detail and not exposed in the bundled js files. Did you intend for the parsed and list-encoded semver ranges to be in the files?

@sokra
Copy link
Member Author

sokra commented Jul 7, 2020

I thought all semver ranges were going to be encoded as strings in the js bundles, and the list encoding was going to be internal implementation detail and not exposed in the bundled js files. Did you intend for the parsed and list-encoded semver ranges to be in the files?

It's not exposed on the container API. loadSingletonVersionCheck is a local method. The container API (for this the share scope) only stores version strings. You can log the share scope if you are interested with console.log(__webpack_share_scopes__.default) (Maybe need to init it before, await __webpack_init_sharing__("default"))

@modgera
Copy link

modgera commented Dec 7, 2022

@sokra
Hello. Can you be so kind and maybe explain why use condition || uniqueName > activeVersion.from in register function in ShareRuntimeModule? It's a little confusing.
Thank you)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants