exploration: automatic code extraction

[maiieul]

What is it?

• Feature / enhancement

Description

Motivation

The optimizer can produce segments code that contain both component code and shared helpers. This can lead to over-preloading on user-interaction or – more problematically – on visible tasks (e.g. instead of ~4-5 bundles to preload, it can easily become ~50-100). This happens because the optimizer doesn't split helpers into separate segments. So in practice if a helper is declared in a separate file, the optimizer will leave it untouched, but if it is colocated in the same file as the component code, then the optimizer will output it in the same facade segment as the componentQrl, meaning that when the helper is imported by another module, not only the helper but also the component and all its transitive dependencies will be eagerly preloaded (100% probability) as well.

Example 1 (helper outside of component$ in output facade segment):

============================= test.js ==

import { componentQrl } from "@qwik.dev/core";
import { qrl } from "@qwik.dev/core";
const i_uFrpfTnm3bA = ()=>import("./test.tsx_TestButton_component_uFrpfTnm3bA");

export const helper = ()=>{ // -> helper function which might be imported anywhere else in the application as `import { helper } from "test.js";`
    console.log("helper");
};

export const TestButton = /*#__PURE__*/ componentQrl(/*#__PURE__*/ qrl(i_uFrpfTnm3bA, "TestButton_component_uFrpfTnm3bA"));

Now this means that if an on-click segment imports { helper } from "./test.js" and TestButton_component_uFrpfTnm3bA has a lot of transitive dependencies, then the the preloader will end up eagerly preloading (100% probability) TestButton_component_uFrpfTnm3bA and all its transitive dependencies, even though we only needed to import helper in the first place.

Here when the visibleTask needs to execute, we will preload the component segment to retrieve the internalHelper$ function along with all the component transitive imports even though we only need to execute internalHelper$.


## Proposal
Even though the developer didn't explicitly mark the helper function as a QRL, it seems necesssary to automatically extract it.

My proposal is to automatically extract each and every helper function declaration for each input given to the optimizer into its own segment like we currently do it for $ functions. Then we 
- group back together the Qwik APIs (tasks, computeds, handlers) like we do today through Rollup based on their shared component (either using `manualChunks` or `emitFile` with `implicitlyLoadedAfterOneOf`)
- let Rollup handle the rest

[stackblitz repro](https://stackblitz.com/edit/rollup-repro-yfegplbc?file=src%2Fshared-dep.js,src%2Fshared-dep3.js,src%2Fshared-dep2.js,rollup.config.js,src%2Fmain.js,src%2Futil1.js,src%2Futil2.js,src%2Fa2.js,src%2Fa1.js,src%2Fa3.js,src%2Fa2-click.js,src%2Fa1-click.js,src%2Fa1-task.js,src%2Fa2-task.js,src%2Fa3-task.js,src%2Fb1.js,src%2Fb1-task.js)

> Note: not grouping back together the Qwik APIs is also possible with a `maxIdleModulepreloads: ~50`. It increases the amount of chunks but also increases caching and reduces cache invalidation (version skew).
 
## Pros & Cons
Pros: 
- No more over-preloading on click or visible tasks
- Better caching and reduced cache invalidation (less version skew) since modifying helpers won't always invalidate component code
- Better bundling since Rollup will be able to bring back together non-shared helpers chains with their importer
- Possibility to get rid of the $ sign

Cons:
- This could lead to more chunks in the final output. Rollup should bring them back together in the most optimal way but this will require testing and adjusting the `maxIdlePreloads` default value. Perhaps we will get better results with `emitFile` with `implicitlyLoadedAfterOneOf` instead of `manualChunks` (see [stackblitz repro](https://stackblitz.com/edit/rollup-repro-yfegplbc?file=src%2Fshared-dep.js,src%2Fshared-dep3.js,src%2Fshared-dep2.js,rollup.config.js,src%2Fmain.js,src%2Futil1.js,src%2Futil2.js,src%2Fa2.js,src%2Fa1.js,src%2Fa3.js,src%2Fa2-click.js,src%2Fa1-click.js,src%2Fa1-task.js,src%2Fa2-task.js,src%2Fa3-task.js,src%2Fb1.js,src%2Fb1-task.js)

## Requirements:

- Optimizer would need a first lightweight pass to collect import aliases and map their `resolve()` value (e.g. `import { helper } from "@alias/helpers"`).
  - I thought that instead of a resolved aliases map, we could keep aliases untouched and leverage javascript `export { helper_hash as helper };`. Thich would work with barrel files, but then there's no way to retrieve `helper` with direct alias path access such as `import { helper } from "@alias/helpers/path_to_helper/helper";` which would try to get `helper` from the original file but it won't be there anymore. 


<!--
* Include a summary of the motivation and context for this PR
* Is it related to any opened issues? (please add them here)
-->

# Checklist

<!--
* delete the items that are not relevant, so it's easy to tell if the PR is ready to be merged
* add items that are relevant and need to be done before merging
-->

- [ ] My code follows the [developer guidelines of this project](https://github.com/QwikDev/qwik/blob/main/CONTRIBUTING.md)
- [ ] I performed a self-review of my own code
- [ ] I added a changeset with `pnpm change`
- [ ] I made corresponding changes to the Qwik docs
- [ ] I added new tests to cover the fix / functionality

[changeset-bot]

⚠️ No Changeset found

Latest commit: 2852342

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

[maiieul]

All hashes are computed based on fileName + functionName + canonicalPath. This works for aliases and barrel files too since the we would do a first pass to map the aliases with their resolved value.

[maiieul]

This facade segment is kept by the optimizer to not process aliases and simply allow javascript to resolve them on its own. It therefore contains any top level import of the original file which leads to overpreloading. So we need to remove it and use a different approach.

[maiieul]

Not strictly necessary but all functions are named with a hash to keep the algorithm simple

[maiieul]

all helper functions are moved into their own segment

[maiieul]

even internalHelper$ inside TestButton gets extracted into its own segment

[maiieul]

aliasHelper can be transformed since we have the resolved aliases map

[maiieul]

Even though internalHelper$is a qrl inside TestButton, it will lead to overpreloading from a click segment because we will need to get the code for internalHelper$ from test_tsx_TestButton_component_uFrpfTnm3bA anyways and therefore preload all of the transitive deps here as well.

[maiieul]

No need to hold a reference to internalHelper$ anymore since it's extracted and imported as a separate segment

[maiieul]

Even though onlyOneImporterHelper is in a separate segment, it will be bundled back together with test_TestButton_component_useVisibleTask_DHnqAQBxHLQ with Rollup.

[maiieul]

Even external QRLs aren't extracted into their own segments and can therefore lead to over-preloading when invoked

[github-actions]

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
qwik-docs	✅ Ready (View Log)	Visit Preview	ad5bc0a

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@8284

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@8284

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@8284

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@8284

commit: ad5bc0a