Add no-op tier import API #9865

JakeLane · 2024-07-23T07:30:02Z

↪️ Pull Request

Basic implementation of the API for tiered imports behind a feature flag. Raising this PR early to gain feedback on API and how we should include changes to the runtime code.

JakeLane · 2024-07-23T07:32:24Z

packages/bundlers/default/src/DefaultBundler.js

@@ -277,7 +278,8 @@ function decorateLegacyGraph(
        );
        for (let incomingDep of incomingDeps) {
          if (
-            incomingDep.priority === 'lazy' &&
+            (incomingDep.priority === 'lazy' ||


This is treated as "lazy" for now. I think tier will need some special treatment as we need to act like a 'parallel' priority asset in most cases.

If this is a noop for now, then it should act the same as a hard require, not a lazy right?

(or are you doing more than nooping?)

This is updated now to use the packager to make the imports sync

JakeLane · 2024-07-23T07:34:03Z

packages/examples/phases/src/index.tsx

+import ModulePhase1 from './phase1';
+import {deferredLoadComponent} from './utils';
+const Phase2 = deferredLoadComponent(
+  importDeferredForDisplay<typeof import('./phase2')>('./phase2'),


The type here will need to be enforced with linting. If we add an autofixer, it should be pretty easy to use for devs.

JakeLane · 2024-07-23T07:35:01Z

packages/examples/phases/src/utils.tsx

+  return function WrappedComponent(props) {
+    useEffect(() => {
+      return () => {
+        cleanUp?.();


Important otherwise we get memory leaks, no keen on this as it looks like a dangerous pattern. While people probably don't want to be using the imports directly often, I'd be happy if someone can suggest a better way of cleaning up automatically

JakeLane · 2024-07-23T07:36:26Z

packages/examples/phases/src/utils.tsx

+      };
+    }, []);
+    if (loaded) {
+      return <Resource.mod.default {...props} />;


We'll need to document that we require a default export. Alternatively we could pass in a key to map to the correct import, but that's messy business I'd like to avoid

JakeLane · 2024-07-23T08:09:19Z

packages/runtimes/js/src/JSRuntime.js

  }

  let code = [];

  if (needsEsmLoadPrelude) {
    code.push(`let load = require('./helpers/browser/esm-js-loader');`);
  }
+  if (needsTierPrelude) {
+    code.push(`let tier = require('./helpers/browser/tier-loader');`);


This code only exists for now in "lazy" priority code (same as doing a dynamic import('x')), this is one of the things that will need to change when we start thinking about "tiered" code possibly existing inline or external.

tomgasson · 2024-07-23T07:54:02Z

packages/packagers/js/src/ScopeHoistingPackager.js

@@ -36,6 +36,8 @@ import {
 } from './utils';
 // General regex used to replace imports with the resolved code, references with resolutions,
 // and count the number of newlines in the file for source maps.
+const REPLACEMENT_RE_TIERED =
+  /\n|import\s+"([0-9a-f]{16}:.+?)";|(?:\$[0-9a-f]{16}\$exports)|(?:\$[0-9a-f]{16}\$(?:import|importAsync|require|importDeferredForDisplay|importDeferred)\$[0-9a-f]+(?:\$[0-9a-f]+)?)/g;


Regex replacements aren't ideal. Good if we can shift this to be an AST operation at some point

tomgasson · 2024-07-23T07:54:41Z

crates/parcel_core/src/types/dependency.rs

@@ -175,6 +175,8 @@ pub enum Priority {
  Parallel = 1,
  /// The dependency should be placed in a separate bundle that is loaded later
  Lazy = 2,
+  /// The dependency should be deferred to a different tier
+  Tier = 3,


nit: Call this Deferred

tomgasson · 2024-07-23T07:56:31Z

packages/bundlers/default/src/DefaultBundler.js

@@ -277,7 +278,8 @@ function decorateLegacyGraph(
        );
        for (let incomingDep of incomingDeps) {
          if (
-            incomingDep.priority === 'lazy' &&
+            (incomingDep.priority === 'lazy' ||


If this is a noop for now, then it should act the same as a hard require, not a lazy right?

(or are you doing more than nooping?)

tomgasson · 2024-07-23T08:01:02Z

packages/examples/phases/phases.d.ts

+
+declare function importDeferred<T extends any | void = void>(
+  source: T extends void ? ErrorMessage : ModuleRef<T>,
+): DeferredImport<T>;


I recommend that these only provide access to the default exported value, not the module itself.

It's the bundlers responsibility to ensure that devs can't shoot themselves in the foot by doing things like importing the one file both deferred and non-deferred because they're putting multiple values in the one file.

tomgasson · 2024-07-23T08:04:31Z

packages/examples/phases/src/index.tsx

+
+import ModulePhase1 from './phase1';
+import {deferredLoadComponent} from './utils';
+const Phase2 = deferredLoadComponent(


Encourage you to not nest these even in examples.

const Phase2Deferred = importDeferredForDisplay<typeof import('./Phase2')>('./Phase2'); const Phase2 = deferredLoadComponent(Phase2Deferred)

If we ever get a syntax spec'd which is compatible with what we're after here, it'll be a top-level declaration. We should also lint so that these APIs are only called on the top-level (avoids memoisation issues and the idea that they're doing something at runtime).

deferredLoadComponent however is a runtime function

Extra thought about this: by keeping them at the top level we can avoid needing to explore all call expressions and instead only need to tackle the immediate children of the top-level of the module.

Allows us to shortcut the work needed for dependency-extraction (though require will always hold that back)

tomgasson · 2024-07-23T12:23:14Z

packages/examples/phases/src/utils.tsx

+    if (loaded) {
+      return <Resource.mod.default {...props} />;
+    } else {
+      throw new Promise(resolve => {


This promise needs to be cached, so that re-renders get the same promise instance

tomgasson · 2024-07-23T12:28:12Z

packages/examples/phases/phases.d.ts

+interface DeferredImport<T> {
+  onReady(resource: () => void): () => void;
+  mod: T | null;
+}


interface DeferredImport<T> { onReady(callback: (T) => void): void; }

No need for a cleanup. No bundler cleans up their already loaded JS modules (AFAIK), and if they do they'll set a precedent of how runtime code should drop references.

The content is the default value, not the EM module's whole scope. It's so, so, so much easier that way. Means we avoid partial-value caching (remember that caching a promise / callback-able thing means we've caching not only the end value but also the promise / work to get it, else multiple of them will be constructed)

No need to expose the module value directly. You need to onReady() to get access to it

tomgasson · 2024-07-23T12:30:27Z

packages/examples/phases/src/utils.tsx

+
+export function deferredLoadComponent<T>(
+  Resource: DeferredImport<T extends {default: any} ? T : never>,
+): FC {


This needs to cache (using a WeakMap) based key'd by the DeferredImport, for both re-renders but also so that multiple components both rendering the same deferred component will be the same thing

tomgasson · 2024-07-23T12:32:04Z

packages/transformers/js/core/src/dependency_collector.rs

@@ -49,6 +49,16 @@ pub enum DependencyKind {
  /// import('./dependency').then(({x}) => {/* ... */});
  /// ```
  DynamicImport,
+  /// Corresponds to a for display (tier) import statement
+  /// ```skip
+  /// const {x} = importDeferredForDisplay('./dependency');


Suggested change

/// const {x} = importDeferredForDisplay('./dependency');

/// const DependencyDeferred = importDeferredForDisplay('./dependency');

Can't destructure this, it's a class instance.

tomgasson · 2024-07-23T12:32:09Z

packages/transformers/js/core/src/dependency_collector.rs

+  DeferredForDisplayTierImport,
+  /// Corresponds to a deferred (tier) import statement
+  /// ```skip
+  /// const {x} = importDeferred('./dependency');


Can't destructure this, it's a class instance.

devongovett · 2024-07-24T00:34:25Z

packages/transformers/js/core/src/utils.rs

@@ -168,6 +168,34 @@ pub fn match_require(node: &ast::Expr, unresolved_mark: Mark, ignore_mark: Mark)
  }
 }

+/// This matches an expression like `importDeferredForDisplay('id')` or `importDeferred('id')` and returns the dependency id.


Can you use import attributes for this?

import('id', {with: {tier: 'display'}})

This syntax is now supported by TS and other tooling as well so should be safe to use. The advantage over individual functions is that the priorities aren't hard coded into the transformer, and no special TS declarations for these new non-standard global functions would be needed. Internally, you'd also not need any special dependency types or changes to core data structures, and could just use the import attributes stored on the dependency for the metadata.

I evaluated this earlier because it would definitely be easier to ship. Unfortunately we're unable to use it yet as there's no way to safely type it (microsoft/TypeScript#46135). I also saw some issues pop up in Jira with our tooling crashing out which made it more time consuming to implement (SWC wasn't happy, nor was Jest).

I'm keen to migrate over though when the type support is added

Maybe I misunderstood the purpose of this feature, but why does the type of the returned module need to be different from a normal import? I assumed it was just about prioritizing the load order.

I'll give a quick overview for the motivation and approach for the project:

Tiers are a concept that allows us to reason about code in different stages of the page load so that developers can have full control over when the user is loading and executing their code. Web vitals give us guidance on what stages we need to consider for user experience, and we can have corresponding tiers.

First is the “loading” tier, where we need to make clear to the users that we’re working on loading their content. We already achieved this pattern with “skeletons”, but we can make this much more efficient by making a specific tier for this and showing the loading state as early as possible. This corresponds to sync imports.

Second is the “visually complete” tier, where the focus is on ensuring users can see everything they need and start interacting with the experience. This tier can be considered SSR returning most of the page as well as any React code required to enable basic interactivity in the page, such that users can make interactions that are actioned when the required code has loaded. This corresponds to importDeferredForDisplay.

Third is the “fully interactive” state, where users can interact fully with the experience without waiting for deferred content to appear. Less critical code, such as logging and background tasks, should be executed in this tier. Importantly, as the visually complete tier has already happened, we should not trigger a paint. We should expect no visual changes during this tier and a Cumulative Layout Shift of ~0. This corresponds to importDeferred.

These new imports need to return a different type so we can treat them differently at runtime, especially for using stuff like suspense (like in the example). Stuff like react-loosely-lazy is what we used before, but this isn't as efficient nor bundler aware. Each tier we want to have a stage of "blocked" and "unblocked" so that each tier cannot interfere with another, which happens with react-loosely-lazy. That's important because in our current solution, we end up with microtasks from critical code being run after non-critical code (such as stuff that doesn't paint or just stuff like analytics).

I'm still not sure why that would require a special type. Couldn't the priority queue be internal to the runtime and not exposed to user code?

The main issue we have here is we want sync loading if the module is available and promise loading if it's not (like https://relay.dev/docs/glossary/#observable). The APIs available to us right now don't allow this, it's one or the other (import x from 'y' or x = import('y')). Happy to chat about this in more depth in the core catch up

I believe React supports this out of the box using some extensions to the Promise object, namely status, value, and reason. See the RFC for use() here: https://github.com/acdlite/rfcs/blob/first-class-promises/text/0000-first-class-support-for-promises.md#reading-the-result-of-a-promise-that-was-read-previously.

Maybe you can use something similar? It would be really nice to avoid adding new APIs.

yamadapc · 2024-08-01T00:35:29Z

packages/transformers/js/core/src/dependency_collector.rs

There are exhaustive tests on this file, could you update them?

yamadapc · 2024-08-01T00:35:53Z

crates/parcel_plugin_transformer_js/src/transformer/conversion/dependency_kind.rs

Please update the tests with your changes

yamadapc · 2024-08-01T00:36:32Z

packages/transformers/js/core/src/utils.rs

+  use ast::*;
+
+  match node {
+    Expr::Call(call) => match &call.callee {


you can collapse all of this onto just one expression I think

…ehaviour

JakeLane commented Jul 23, 2024

View reviewed changes

github-actions bot deployed to Preview July 23, 2024 07:41 View deployment

JakeLane commented Jul 23, 2024

View reviewed changes

tomgasson reviewed Jul 23, 2024

View reviewed changes

devongovett reviewed Jul 24, 2024

View reviewed changes

yamadapc reviewed Aug 1, 2024

View reviewed changes

JakeLane force-pushed the feat/base-tiered-imports branch from 689a5f1 to 99574ef Compare August 1, 2024 07:30

JakeLane changed the title ~~WIP DO NOT REVIEW - Add no-op tier import API~~ Add no-op tier import API Aug 5, 2024

JakeLane added 5 commits August 5, 2024 16:03

Add no-op tier import API

f84f390

Refactor to use packager to handle runtime code

5403fc9

Refactor code to transformer and update example for better rerender b…

d4e043b

…ehaviour

Update examples for memoisation and move back to packager

7c76b0c

Add tests for dependency kind

bfd803b

JakeLane force-pushed the feat/base-tiered-imports branch from bf7e6f4 to bfd803b Compare August 5, 2024 06:16

github-actions bot deployed to Preview August 5, 2024 06:27 View deployment

Remove modern syntax from packager

7a18e5d

github-actions bot deployed to Preview August 5, 2024 06:37 View deployment

Fix test referencing old symbol

4ad1af2

github-actions bot deployed to Preview August 6, 2024 00:21 View deployment

Use if instead of match to simplify tier match

f6a1411

github-actions bot deployed to Preview August 6, 2024 05:57 View deployment

JakeLane added 2 commits August 7, 2024 09:06

Fix not returning string match

0f1aeca

Move to unsafe_ API

ca5b78b

devongovett closed this Aug 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add no-op tier import API #9865

Add no-op tier import API #9865

JakeLane commented Jul 23, 2024

JakeLane Jul 23, 2024

tomgasson Jul 23, 2024

JakeLane Jul 30, 2024

JakeLane Jul 23, 2024

JakeLane Jul 23, 2024

JakeLane Jul 23, 2024

JakeLane Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

tomgasson Jul 23, 2024

devongovett Jul 24, 2024

JakeLane Jul 24, 2024 •

edited

Loading

devongovett Jul 24, 2024

JakeLane Jul 25, 2024

devongovett Jul 29, 2024

JakeLane Aug 5, 2024

devongovett Aug 5, 2024

yamadapc Aug 1, 2024

yamadapc Aug 1, 2024

yamadapc Aug 1, 2024

	/// const {x} = importDeferredForDisplay('./dependency');
	/// const DependencyDeferred = importDeferredForDisplay('./dependency');

Add no-op tier import API #9865

Add no-op tier import API #9865

Conversation

JakeLane commented Jul 23, 2024

↪️ Pull Request

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JakeLane Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JakeLane Jul 24, 2024 •

edited

Loading