Sync dynamic import transformer #876

at-nathan · 2025-11-10T20:28:22Z

Motivation

Provide native functionality parity for handling dynamic imports, mimicking its Babel counterpart.

Changes

Adds a transformer that converts dynamic imports to a require statement for configured paths and EntryPoints, or a dummy noop Promise by default.

Checklist

Existing or new tests cover this change
There is a changeset for this change, or one is not required
Added documentation for any new features to the docs/ folder

changeset-bot · 2025-11-10T20:28:26Z

🦋 Changeset detected

Latest commit: 4c62690

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

This PR includes changesets to release 9 packages

Name	Type
@atlaspack/transformer-js	Patch
@atlaspack/config-default	Patch
atlaspack	Patch
@atlaspack/config-webextension	Patch
@atlaspack/cli	Patch
@atlaspack/register	Patch
@atlaspack/test-utils	Patch
@atlaspack/inspector	Patch
@atlaspack/inspector-frontend	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

JakeLane · 2025-11-10T22:53:18Z

packages/transformers/js/src/JSTransformer.ts

      options.env.NATIVE_LAZY_LOADING_TRANSFORMER,
    );
+    let syncDynamicImportConfig =
+      options.env.SYNC_DYNAMIC_IMPORT_CONFIG || undefined;


Might be worth adding an integration test?

marcins · 2025-11-11T02:12:48Z

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

+                        unresolved_mark,
+                        config.sync_dynamic_import_config.clone(),
+                      )),
+                      config.sync_dynamic_import_config.is_some()),


Should we also only allow this visitor if the context is Tesseract?

marcins · 2025-11-11T02:15:03Z

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

+  ) -> Self {
+    let config: SyncDynamicImportConfig = json_string_config
+      .as_ref()
+      .and_then(|json_str| serde_json::from_str::<SyncDynamicImportConfig>(json_str).ok())


This deserialisation will be done for every file - is there any performance issues with that or is this fairly fast? The alternative is to do it in loadConfig and pass an object in to the visitor, though that'll also occur for every file due to the way Transformer config currently works.

Good call out, I assumed that serde_json would be faster than JSON.parse. For loadConfig could I store the result in global? Or does Rust/Native Atlaspack also a global or singleton I can store the result in for the Atlaspack instance?

I think the main consideration for me, even if it's not faster, is that Matt is working on some caching stuff that will affect loadConfig - some of that will be moved to a new setup type method that does only run once. Doing it in loadConfig now will be closer to that final state than having it hidden away in the Rust visitor.

In terms of caching the result, I don't think there's currently any blessed/"safe" way if it's not already available in the config API (which mostly deals with loading config from files, rather than from the environment). You should probably use the config API though, and the invalidateOnEnvChange to register an invalidation for when that environment variable changes.

I guess you could consider using a [buildCache](https://github.com/atlassian-labs/atlaspack/blob/main/packages/core/build-cache/src/buildCache.ts#L3) - which provides a mechanism for a KV cache (a Map really), that gets cleared before each build.

So to summarise, now that I've thought about it:

make sure you call config.invalidateOnEnv('ENV_NAME') for any config you read from options.env in the JSTransformer

Parse in JSTransformer::loadConfig as this will eventually be done once (it's also clearer what's happening than having it hidden deep in a visitor)

Consider using a buildCache for caching the JSON.parse result (i.e. you could use the source string as the key, and the parsed config as the value) - this automatically gets cleared between builds.

marcins · 2025-11-11T02:15:19Z

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

+    }
+  }
+
+  fn __override_resolve_module_path(&mut self, override_fn: Option<fn(&str) -> Option<String>>) {


Why is this function named with a __?

It's meant to be an internal method for allowing tests to pass in a mocked version of file resolving. Was hoping the double _ would symbolise that. I can remove if it's confusing.

I think generally in Rust a _ prefix denotes unused. If a method is only required for testing you can actually add an annotation that will only compile it in for test builds, e.g.:

#[cfg(test)] fn override_resolve_module_path(..)

.. the method won't exist outside of tests.

Is the method even necessary instead of just directly setting the property where currently called?

- Add config cache - Add tests

packages/transformers/js/src/JSTransformer.ts

at-nathan · 2025-11-20T03:37:27Z

packages/transformers/js/src/JSTransformer.ts

+      }
+    }
+
+    config.invalidateOnEnvChange('SYNC_DYNAMIC_IMPORT_CONFIG');


Seeking feedback on if this is the right place to call config.invalidateOnEnvChange, I'm not too familiar with the API.

I think this is right? @mattcompiles, can you sanity check that for me? It's the config that gets invalidate when the env changes.. this shouldn't be asset.invalidateOnEnvChange in transform, right?

Yeah that's right, it'll invalidate the Asset anyway though.

github-actions · 2025-11-21T11:28:14Z

📊 Type Coverage Report

Coverage Comparison

Metric	Baseline	Current	Change
Coverage Percentage	92.07%	92.07%	➡️ 0.00%
Correctly Typed	203,025	203,200	+175
Total Expressions	220,492	220,683	+191
Untyped Expressions	17,467	17,483	+16

Files with Most Type Issues (Top 15)

File	Issues	Affected Lines
`packages/core/integration-tests/test/javascript.ts`	999	658
`packages/core/integration-tests/test/cache.ts`	885	626
`packages/core/integration-tests/test/scope-hoisting.ts`	623	490
`packages/utils/node-resolver-core/test/resolver.ts`	476	177
`packages/core/integration-tests/test/html.ts`	468	294
`packages/core/integration-tests/test/sourcemaps.ts`	356	176
`packages/core/test-utils/src/utils.ts`	330	205
`packages/core/integration-tests/test/incremental-bundling.ts`	298	206
`packages/core/core/src/dumpGraphToGraphViz.ts`	251	108
`packages/core/integration-tests/test/output-formats.ts`	227	161
`packages/transformers/webextension/src/WebExtensionTransformer.ts`	210	80
`packages/core/integration-tests/test/css-modules.ts`	191	107
`packages/core/core/src/requests/TargetRequest.ts`	190	133
`packages/core/integration-tests/test/react-refresh.ts`	188	65
`packages/core/integration-tests/test/babel.ts`	187	115

This report was generated by the Type Coverage GitHub Action

github-actions · 2025-11-21T11:43:54Z

📊 Benchmark Results

✅ No significant performance changes detected.

📊 Benchmark Results

Overall Performance

Test	Duration	JS Memory Peak	Native Memory Peak	vs Baseline	Status
Three.js Real Repository (JS)	16.72s	1.92GB	1.96GB	N/A	⚪ New
Three.js Real Repository (Native)	27.91s	3.40GB	3.71GB	N/A	⚪ New

🔍 Detailed Phase Analysis

Three.js Real Repository (JS)

Phase	Duration (avg)	Duration (p95)	Memory Peak (avg)	Memory Peak (p95)
resolving	8.94s	9.02s	1.29GB	1.45GB
transforming	8.93s	9.02s	1.29GB	1.45GB
bundling	8.69s	8.81s	1.29GB	1.45GB
bundled	8.24s	8.39s	1.40GB	1.56GB
packaging	6.11s	6.25s	1.55GB	1.66GB
optimizing	5.97s	6.09s	1.92GB	1.98GB

Three.js Real Repository (Native)

Phase	Duration (avg)	Duration (p95)	Memory Peak (avg)	Memory Peak (p95)
bundling	18.26s	24.96s	2.51GB	2.86GB
bundled	17.84s	24.54s	2.68GB	3.04GB
packaging	8.22s	8.51s	2.95GB	3.28GB
optimizing	8.07s	8.41s	3.40GB	3.71GB

💾 Unified Memory Analysis

Three.js Real Repository (JS) Memory Statistics

Memory Type	Metric	Min	Mean	Median	P95	P99	Max	Std Dev
JavaScript	RSS	1.42GB	1.72GB	1.81GB	1.98GB	1.98GB	1.98GB	215.25MB
	Heap Used	80.19MB	86.06MB	88.25MB	93.31MB	93.31MB	93.31MB	5.76MB
	Heap Total	94.35MB	128.63MB	126.68MB	185.12MB	185.12MB	185.12MB	28.86MB
	External	38.95MB	106.05MB	173.09MB	173.16MB	173.16MB	173.16MB	67.06MB
Native (Rust)	Physical Memory	1.36GB	1.59GB	1.62GB	1.80GB	1.92GB	1.96GB	150.38MB
	Virtual Memory	30.18GB	30.86GB	30.88GB	31.10GB	31.22GB	31.33GB	201.31MB

Sample Counts: JS: 6, Native: 268

Three.js Real Repository (Native) Memory Statistics

Memory Type	Metric	Min	Mean	Median	P95	P99	Max	Std Dev
JavaScript	RSS	2.56GB	3.14GB	3.21GB	3.71GB	3.71GB	3.71GB	378.33MB
	Heap Used	76.58MB	77.06MB	77.22MB	77.51MB	77.51MB	77.51MB	0.33MB
	Heap Total	87.95MB	93.28MB	89.95MB	103.70MB	103.70MB	103.70MB	6.56MB
	External	179.54MB	182.45MB	185.32MB	185.35MB	185.35MB	185.35MB	2.89MB
Native (Rust)	Physical Memory	2.70GB	3.08GB	3.09GB	3.46GB	3.71GB	3.71GB	234.67MB
	Virtual Memory	30.73GB	33.38GB	33.45GB	33.81GB	34.06GB	34.06GB	513.21MB

Sample Counts: JS: 6, Native: 665

🖥️ Environment

Node.js: v22.17.0
Platform: linux (x64)
CPU: AMD EPYC 7763 64-Core Processor
Total Memory: 15.62GB
Git SHA: 0a51f90
Branch: sync-dynamic-import-transform
Timestamp: 2025-11-24T06:41:18.792Z

mattcompiles · 2025-11-23T23:52:26Z

packages/transformers/js/src/JSTransformer.ts

+          invariant(typeof config?.entrypoint_filepath_suffix === 'string');
+          invariant(Array.isArray(config.actual_require_paths));
+
+          configCache.set('SYNC_DYNAMIC_IMPORT_CONFIG', config);


I think we're reinventing cached config reading here a little. I think we should just use config.getConfigFrom rather than using a custom build cache.

Interesting, so you're suggesting we should put this config inside a separate file for getConfigFrom to read? Does this deviate from our upcoming dynamic config work where we want all config to be centralised?

Sorry I've misread this slightly. The env var you're passing is the config, not a file that contains it which was my confusion. Build caches are a little notorious for causing problems so be careful 😅

mattcompiles · 2025-11-23T23:53:58Z

packages/transformers/js/src/JSTransformer.ts

    let enableLazyLoadingTransformer = Boolean(
      options.env.NATIVE_LAZY_LOADING_TRANSFORMER,
    );
+    let syncDynamicImportConfig:


Is this config a temporary solution or a permanent one? As all this code won't run when atlaspackV3 is enabled.

I'm leaning towards temporary until we have dynamic config, the reason why we're adding it to options.env is so we can feature flag the addition of config, where this was not possible with package.json. Are we planning on consuming atlaspackV3 soon? If so could you please point me to the transformer-js config equivalent?

The V3 version of this file is located here: crates/atlaspack_plugin_transformer_js/src/js_transformer.rs
We're already using this for production Jira builds and we're rolling it out to more products over the next month.

packages/transformers/js/src/JSTransformer.ts

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

OscarCookeAbbott · 2025-11-24T04:23:42Z

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

+  ) -> Self {
+    let config: SyncDynamicImportConfig = json_string_config
+      .as_ref()
+      .and_then(|json_str| serde_json::from_str::<SyncDynamicImportConfig>(json_str).ok())


I can't remember, what was the reasoning for deserialising in Rust instead of JS? Because the latter would be simpler of course

I'm not sure why GitHub marked this comment as outdated

This code no longer exists, please check latest changes.

Yep I can see it is gone, that means there are some dangling serde and probably other imports that can now be removed too.

OscarCookeAbbott · 2025-11-24T04:25:17Z

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

+    }
+  }
+
+  fn __override_resolve_module_path(&mut self, override_fn: Option<fn(&str) -> Option<String>>) {


Is the method even necessary instead of just directly setting the property where currently called?

OscarCookeAbbott · 2025-11-24T04:25:48Z

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

+    })
+  }
+
+  fn create_dummy_promise(&self) -> Expr {


You may be able to use quote!() for these kinds of AST construction

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

OscarCookeAbbott · 2025-11-25T01:49:29Z

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

+                        unresolved_mark,
+                        &config.sync_dynamic_import_config,
+                      )),
+                      config.is_tesseract && config.sync_dynamic_import_config.is_some()),


Rather than pass is_tesseract all the way down the chain into Rust, you could mimic what was done with is_worker where the combined check is made in JS so Rust doesn't itself know anything.

- Support V3

at-nathan added 3 commits November 7, 2025 13:39

Initial commit

5dd1859

- Add config intake

9f7363d

- Changeset

c95d6eb

at-nathan requested review from OscarCookeAbbott, marcins and vykimnguyen November 10, 2025 20:28

at-nathan requested a review from a team as a code owner November 10, 2025 20:28

at-nathan self-assigned this Nov 10, 2025

JakeLane reviewed Nov 10, 2025

View reviewed changes

JakeLane approved these changes Nov 10, 2025

View reviewed changes

marcins approved these changes Nov 11, 2025

View reviewed changes

- Emulate resolve

6a2ef32

- Add config cache - Add tests

at-nathan commented Nov 20, 2025

View reviewed changes

packages/transformers/js/src/JSTransformer.ts Show resolved Hide resolved

at-nathan commented Nov 20, 2025

View reviewed changes

Merge branch 'main' into sync-dynamic-import-transform

1668bbe

Merge branch 'main' into sync-dynamic-import-transform

5d22ac7

mattcompiles reviewed Nov 23, 2025

View reviewed changes

OscarCookeAbbott mentioned this pull request Nov 24, 2025

Implement native react_async_import_lift transformer #902

Draft

3 tasks

OscarCookeAbbott reviewed Nov 24, 2025

View reviewed changes

packages/transformers/js/src/JSTransformer.ts Outdated Show resolved Hide resolved

packages/transformers/js/core/src/sync_dynamic_import.rs Show resolved Hide resolved

OscarCookeAbbott requested changes Nov 24, 2025

View reviewed changes

at-nathan added 2 commits November 24, 2025 17:20

Clippy suggestions

a814fc0

More Clippy suggestions

0a51f90

OscarCookeAbbott requested changes Nov 25, 2025

View reviewed changes

- Move isTesseract check

4c62690

- Support V3

Sync dynamic import transformer #876

Are you sure you want to change the base?

Sync dynamic import transformer #876

Uh oh!

Conversation

at-nathan commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Checklist

Uh oh!

changeset-bot bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Type Coverage Report

Coverage Comparison

Files with Most Type Issues (Top 15)

Uh oh!

github-actions bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Benchmark Results

📊 Benchmark Results

Overall Performance

🔍 Detailed Phase Analysis

Three.js Real Repository (JS)

Three.js Real Repository (Native)

💾 Unified Memory Analysis

Three.js Real Repository (JS) Memory Statistics

Three.js Real Repository (Native) Memory Statistics

🖥️ Environment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

at-nathan commented Nov 10, 2025 •

edited

Loading

changeset-bot bot commented Nov 10, 2025 •

edited

Loading

github-actions bot commented Nov 21, 2025 •

edited

Loading

github-actions bot commented Nov 21, 2025 •

edited

Loading