[labs/ssr] Add "module" to condition name to resolve for Module Loader #3849

augustjk · 2023-04-26T21:46:35Z

This was causing errors in lit.dev repo running eleventy plugin in VM mode.

This was when trying to load tslib which has these package exports

    "exports": {
        ".": {
            "module": "./tslib.es6.js",
            "import": "./modules/index.js",
            "default": "./tslib.js"
        },

and we actually want the "module" export, as the "import" one is causing the error above.

changeset-bot · 2023-04-26T21:46:38Z

🦋 Changeset detected

Latest commit: e67b108

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@lit-labs/ssr	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-04-26T21:47:01Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -7% - +5% (-1.44ms - +1.05ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 94.18ms - 101.26ms
lit-html-kitchen-sink: unsure 🔍 -6% - +13% (-2.12ms - +4.68ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -5% - +7% (-0.65ms - +0.92ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -3% - +4% (-1.94ms - +2.69ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -6% - +1% (-3.72ms - +0.87ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 956.57ms - 975.69ms
lit-html-kitchen-sink: unsure 🔍 -7% - +3% (-7.08ms - +2.96ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +3% (-8.39ms - +11.07ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +5% (-2.19ms - +6.88ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +1% (-23.22ms - +14.25ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 928.77ms - 950.23ms
reactive-element-list: unsure 🔍 -2% - +1% (-20.52ms - +14.64ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 112.0.5615.165
Sample size: 50
Built by: Benchmarks #4702
Commit: 39abc6c

render

Version	Avg time	vs
	94.18ms - 101.26ms	-

update

Version	Avg time	vs
	956.57ms - 975.69ms	-

update-reflect

Version	Avg time	vs
	928.77ms - 950.23ms	-

lit-html-kitchen-sink

Browser: chrome-headless 112.0.5615.165
Sample size: 30
Built by: Benchmarks #4702
Commit: 39abc6c

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	35.47ms - 41.43ms	-	unsure 🔍 -6% - +13% -2.12ms - +4.68ms	unsure 🔍 -5% - +12% -1.96ms - +4.52ms
tip-of-tree tip-of-tree	35.53ms - 38.81ms	unsure 🔍 -12% - +5% -4.68ms - +2.12ms	-	unsure 🔍 -6% - +6% -2.08ms - +2.08ms
previous-release previous-release	35.89ms - 38.45ms	unsure 🔍 -12% - +5% -4.52ms - +1.96ms	unsure 🔍 -6% - +6% -2.08ms - +2.08ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	93.16ms - 99.24ms	-	unsure 🔍 -7% - +3% -7.08ms - +2.96ms	unsure 🔍 -2% - +6% -2.02ms - +5.79ms
tip-of-tree tip-of-tree	94.26ms - 102.26ms	unsure 🔍 -3% - +7% -2.96ms - +7.08ms	-	unsure 🔍 -1% - +9% -0.74ms - +8.63ms
previous-release previous-release	91.86ms - 96.76ms	unsure 🔍 -6% - +2% -5.79ms - +2.02ms	unsure 🔍 -9% - +1% -8.63ms - +0.74ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	19.36ms - 21.44ms	-	unsure 🔍 -7% - +5% -1.44ms - +1.05ms	unsure 🔍 -10% - +4% -2.07ms - +0.90ms
tip-of-tree tip-of-tree	19.92ms - 21.27ms	unsure 🔍 -5% - +7% -1.05ms - +1.44ms	-	unsure 🔍 -8% - +4% -1.65ms - +0.87ms
previous-release previous-release	19.92ms - 22.04ms	unsure 🔍 -5% - +10% -0.90ms - +2.07ms	unsure 🔍 -4% - +8% -0.87ms - +1.65ms	-

lit-html-repeat

Browser: chrome-headless 112.0.5615.165
Sample size: 30
Built by: Benchmarks #4702
Commit: 39abc6c

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	12.30ms - 13.43ms	-	unsure 🔍 -5% - +7% -0.65ms - +0.92ms	unsure 🔍 -5% - +7% -0.68ms - +0.93ms
tip-of-tree tip-of-tree	12.19ms - 13.27ms	unsure 🔍 -7% - +5% -0.92ms - +0.65ms	-	unsure 🔍 -6% - +6% -0.81ms - +0.79ms
previous-release previous-release	12.16ms - 13.32ms	unsure 🔍 -7% - +5% -0.93ms - +0.68ms	unsure 🔍 -6% - +6% -0.79ms - +0.81ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	339.69ms - 351.67ms	-	unsure 🔍 -2% - +3% -8.39ms - +11.07ms	unsure 🔍 -4% - +2% -12.74ms - +8.38ms
tip-of-tree tip-of-tree	336.67ms - 352.01ms	unsure 🔍 -3% - +2% -11.07ms - +8.39ms	-	unsure 🔍 -4% - +2% -15.12ms - +8.08ms
previous-release previous-release	339.16ms - 356.56ms	unsure 🔍 -2% - +4% -8.38ms - +12.74ms	unsure 🔍 -2% - +4% -8.08ms - +15.12ms	-

lit-html-template-heavy

Browser: chrome-headless 112.0.5615.165
Sample size: 50
Built by: Benchmarks #4702
Commit: 39abc6c

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	67.08ms - 70.72ms	-	unsure 🔍 -3% - +4% -1.94ms - +2.69ms	unsure 🔍 -4% - +3% -3.09ms - +2.24ms
tip-of-tree tip-of-tree	67.09ms - 69.96ms	unsure 🔍 -4% - +3% -2.69ms - +1.94ms	-	unsure 🔍 -5% - +2% -3.22ms - +1.62ms
previous-release previous-release	67.38ms - 71.28ms	unsure 🔍 -3% - +4% -2.24ms - +3.09ms	unsure 🔍 -2% - +5% -1.62ms - +3.22ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	142.32ms - 148.77ms	-	unsure 🔍 -2% - +5% -2.19ms - +6.88ms	unsure 🔍 -2% - +4% -2.68ms - +6.13ms
tip-of-tree tip-of-tree	140.01ms - 146.39ms	unsure 🔍 -5% - +1% -6.88ms - +2.19ms	-	unsure 🔍 -3% - +3% -5.00ms - +3.75ms
previous-release previous-release	140.82ms - 146.82ms	unsure 🔍 -4% - +2% -6.13ms - +2.68ms	unsure 🔍 -3% - +4% -3.75ms - +5.00ms	-

reactive-element-list

Browser: chrome-headless 112.0.5615.165
Sample size: 50
Built by: Benchmarks #4702
Commit: 39abc6c

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	59.57ms - 62.67ms	-	unsure 🔍 -6% - +1% -3.72ms - +0.87ms	unsure 🔍 -7% - +0% -4.24ms - +0.20ms
tip-of-tree tip-of-tree	60.85ms - 64.24ms	unsure 🔍 -1% - +6% -0.87ms - +3.72ms	-	unsure 🔍 -5% - +3% -2.92ms - +1.74ms
previous-release previous-release	61.54ms - 64.73ms	unsure 🔍 -0% - +7% -0.20ms - +4.24ms	unsure 🔍 -3% - +5% -1.74ms - +2.92ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	989.13ms - 1011.69ms	-	unsure 🔍 -2% - +1% -23.22ms - +14.25ms	unsure 🔍 -3% - +1% -26.63ms - +7.39ms
tip-of-tree tip-of-tree	989.94ms - 1019.85ms	unsure 🔍 -1% - +2% -14.25ms - +23.22ms	-	unsure 🔍 -2% - +1% -24.78ms - +14.51ms
previous-release previous-release	997.30ms - 1022.77ms	unsure 🔍 -1% - +3% -7.39ms - +26.63ms	unsure 🔍 -1% - +2% -14.51ms - +24.78ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	972.91ms - 997.43ms	-	unsure 🔍 -2% - +1% -20.52ms - +14.64ms	unsure 🔍 -2% - +1% -23.51ms - +13.27ms
tip-of-tree tip-of-tree	975.51ms - 1000.71ms	unsure 🔍 -1% - +2% -14.64ms - +20.52ms	-	unsure 🔍 -2% - +2% -20.80ms - +16.43ms
previous-release previous-release	976.59ms - 1003.99ms	unsure 🔍 -1% - +2% -13.27ms - +23.51ms	unsure 🔍 -2% - +2% -16.43ms - +20.80ms	-

_{tachometer-reporter-action v2 for Benchmarks}

AndrewJakubowicz

Brilliant investigation & elegant fix! Is there a way to test this change?

augustjk · 2023-04-26T21:55:50Z

Brilliant investigation & elegant fix! Is there a way to test this change?

Aha, yes! I have manually gone to node_modules/@lit-labs/ssr/lib/module-loader.js in the lit.dev repo, updated this line, and confirmed the dev command to run successfully.

Edit: in it's current state, I also had to add mode: "vm" to to the lit eleventy plugin config option as well.

AndrewJakubowicz · 2023-04-26T21:58:54Z

Nice! Is it possible to add a unit test as well?

augustjk · 2023-04-26T22:28:36Z

@AndrewJakubowicz thanks for keeping me honest, test added!

AndrewJakubowicz · 2023-04-26T22:49:10Z

packages/labs/ssr/src/test/lib/module-loader_test.ts

+    testIndex
+  );
+  const {module, path: modulePath} = result;
+  assert.is(module.namespace.packageValue, 'module');


This is awesome!

justinfagnani · 2023-04-26T22:52:28Z

Looks like this is caused by microsoft/tslib#161 which appears to be stalled.

augustjk · 2023-04-26T22:58:30Z

Looks like this is caused by microsoft/tslib#161 which appears to be stalled.

@justinfagnani I agree this is non-standard on their part but it seems other tools are including "module" in their default conditions too e.g.
https://vitejs.dev/config/shared-options.html#resolve-mainfields
https://webpack.js.org/guides/package-exports/#conditions
https://github.com/rollup/plugins/tree/master/packages/node-resolve/#exportconditions

* [labs/ssr] Add "module" to condition name to resolve for Module Loader (#3849) * Version Packages (#3851) * Reset initial versions for prerelease mode * Update ts-clone-node and lockfile

augustjk added 2 commits April 26, 2023 14:39

Add "module" to condition name to resolve

aa83a48

Add changeset

89e4fac

augustjk requested a review from AndrewJakubowicz April 26, 2023 21:46

augustjk requested a review from justinfagnani as a code owner April 26, 2023 21:46

AndrewJakubowicz reviewed Apr 26, 2023

View reviewed changes

Add test for module package resolution

e67b108

augustjk requested a review from AndrewJakubowicz April 26, 2023 22:28

AndrewJakubowicz reviewed Apr 26, 2023

View reviewed changes

AndrewJakubowicz approved these changes Apr 26, 2023

View reviewed changes

justinfagnani approved these changes Apr 26, 2023

View reviewed changes

augustjk merged commit 02d6a35 into main Apr 26, 2023
5 of 7 checks passed

augustjk deleted the ssr-module-loader branch April 26, 2023 22:58

lit-robot mentioned this pull request Apr 26, 2023

Version Packages #3851

Merged

augustjk mentioned this pull request Apr 27, 2023

[infra] Fix dev command lit/lit.dev#1079

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[labs/ssr] Add "module" to condition name to resolve for Module Loader #3849

[labs/ssr] Add "module" to condition name to resolve for Module Loader #3849

augustjk commented Apr 26, 2023

changeset-bot bot commented Apr 26, 2023 •

edited

github-actions bot commented Apr 26, 2023 •

edited

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

AndrewJakubowicz left a comment •

edited

augustjk commented Apr 26, 2023 •

edited

AndrewJakubowicz commented Apr 26, 2023

augustjk commented Apr 26, 2023

AndrewJakubowicz Apr 26, 2023

justinfagnani commented Apr 26, 2023

augustjk commented Apr 26, 2023

[labs/ssr] Add "module" to condition name to resolve for Module Loader #3849

[labs/ssr] Add "module" to condition name to resolve for Module Loader #3849

Conversation

augustjk commented Apr 26, 2023

changeset-bot bot commented Apr 26, 2023 • edited

🦋 Changeset detected

github-actions bot commented Apr 26, 2023 • edited

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

AndrewJakubowicz left a comment • edited

Choose a reason for hiding this comment

augustjk commented Apr 26, 2023 • edited

AndrewJakubowicz commented Apr 26, 2023

augustjk commented Apr 26, 2023

AndrewJakubowicz Apr 26, 2023

Choose a reason for hiding this comment

justinfagnani commented Apr 26, 2023

augustjk commented Apr 26, 2023

changeset-bot bot commented Apr 26, 2023 •

edited

github-actions bot commented Apr 26, 2023 •

edited

AndrewJakubowicz left a comment •

edited

augustjk commented Apr 26, 2023 •

edited