Handle loading TypeStrip modules in applications using VM loader by kriszyp · Pull Request #241 · HarperFast/harper

kriszyp · 2026-03-17T15:22:16Z

No description provided.

socket-security · 2026-03-17T15:49:54Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	amaro@1.1.8

View full report

dawsontoth

I like the fallbacks. Could amaro be an optional peer dep, maybe?

cb1kenobi · 2026-03-18T15:45:49Z

+		// Use amaro - the library that Node.js uses internally for type stripping
+		// Try to use the internal one first, if we can, otherwise fallback to loading the external one
+		if (!amaro) {
+			amaro = await import('internal/deps/amaro/dist/index').then(


I would not recommend relying on the bundled amaro version just like I don't recommend on relying on the bundled version of undici.

Wouldn't we rather be consistent with with host's typestripping? If there are actually diffs between whatever amaro we are using and node's, it doesn't seem clear to be me that it is desirable to be inconsistent with our host.
But the more salient point is that import('amaro') will load multiple MBs of extra code. For someone complaining about 20KB in another ticket, I thought this would be pretty important consideration ;).

relying on the bundled version of undici

That's literally what we do every time we call (global) fetch isn't it?

I don't think it matters to be consistent with the host's typestripping nearly as much as it matters to be consistent with what we support. The fallback regex gauntlet proves that type stripping isn't rocket science.

I was unaware of how big amaro is (3.23MB), but that's a distribution size thing that is only relevant for install.

At runtime, whether you import amaro or internal/deps/amaro/dist/index, it loads roughly the same amount of code. The only time this becomes an issue is when you enable Node.js' type stripping/transform types, then it would load two different versions of amaro. This is obviously not ideal and why your code is nice. However, right now, we don't enable type stripping, so it's not an issue. If we wait long enough, we can drop Node.js 20, use the internal amaro, and hope the path doesn't change too often.

If we were concerned about runtime memory, then we would just use the regex gauntlet.

Regarding undici, it indeed does power global fetch(), but they are certainly not he same. fetch() was designed for convenience, not performance. If you need maximum performance and more control over streaming data, then you use undici. If you need to do a quick HTTP call with a minimal request/response, you can use fetch().

At runtime, whether you import amaro or internal/deps/amaro/dist/index, it loads roughly the same amount of code.

I see your point; you are saying that by default internal amaro won't be loaded, so should be the same... I think that is true:

> process.memoryUsage() { rss: 56676352, heapTotal: 8712192, heapUsed: 5667384, external: 2472825, arrayBuffers: 10533 } > require('internal/deps/amaro/dist/index') { transformSync: [Getter] } > gc() undefined > process.memoryUsage() { rss: 74571776, heapTotal: 11149312, heapUsed: 9387120, external: 8598955, arrayBuffers: 10705 } > (To exit, press Ctrl+C again or Ctrl+D or type .exit) > kzyp@kzyp-XPS-15-9520:~/dev/harper-pro/core$ node --expose-gc --expose-internals Welcome to Node.js v24.10.0. Type ".help" for more information. > process.memoryUsage() { rss: 56250368, heapTotal: 7663616, heapUsed: 5589456, external: 2472819, arrayBuffers: 10527 } > require('amaro') { transformSync: [Getter] } > gc() undefined > process.memoryUsage() { rss: 72826880, heapTotal: 17776640, heapUsed: 12546496, external: 5141239, arrayBuffers: 10705 }

This is actually really confusing; external amaro adds 3 more MBs of heap than internal amaro. But internal amaro uses more "external" memory, and I guess they end up being the same.

what we support

But we don't explicitly support anything specific in TypeStrip, we just vaguely support TypeStrip as a general principle, which is exactly the level of vagueness the internal module provides.

But anyway, pretty easy to go with amaro as a package. Not fully convinced of the benefit, but I can switch that, maybe its slightly safer.

cb1kenobi · 2026-03-18T15:47:02Z

+			);
+			amaro ??= await import('amaro').catch(() => null);
+		}
+		if (amaro?.transformSync) {


If we use amaro from npm, we can guarantee that transformSync exists.

As a bonus, we can strongly type the amaro variable above.

cb1kenobi · 2026-03-18T15:47:36Z

+
+	// Fall back to basic regex-based stripping
+	// Remove type annotations from parameters and variables
+	source = source.replace(/:\s*[A-Za-z_$][\w$<>[\]|&,\s]*(?=[,)\]=;])/g, '');


Is this for Node.js 20? Node.js 20 becomes EOS in like 4 weeks.

Cool. I'll remove in 4 weeks. Although no guarantees that we won't need to limp someone along on v20, obviously has happened before.

Co-authored-by: Chris Barber <chris@harperdb.io>

Ethan-Arrowood

If it works, I'm okay with it.

I think the regex is a bit excessive. Realistically, when would we fail to load the external amaro? I think if that happens we should properly error not fallback to some alternate less-effective procedure. They UX will just not be the same

Ethan-Arrowood

Lol you updated it as soon as I reviewed.

Well now I'm even happier with it!

Just want to assert we are okay with the given error experience; otherwise LGTM

Ethan-Arrowood · 2026-03-18T16:50:50Z

+	amaro = await import('amaro').catch(() => null);
+	return amaro.transformSync(source, { mode: 'strip-only' }).code;


Are we okay with this throwing a TypeError: Cannot read properties of null when the package fails to load?

Its a real dependency now, and Chris removed the unnecessary catch, so I think we are good.

Co-authored-by: Chris Barber <chris@harperdb.io>

cb1kenobi

Ship it!

kriszyp added 2 commits March 17, 2026 09:21

Handle loading TypeStrip modules in applications using VM loader

9ae16c0

Directly use amaro for type stripping

52ab127

kriszyp force-pushed the vm-load-typescript branch from 1438b23 to 52ab127 Compare March 17, 2026 15:41

Use internal amaro if available, otherwise load external amaro

fe3f544

kriszyp force-pushed the vm-load-typescript branch from a727190 to fe3f544 Compare March 17, 2026 15:49

kriszyp added 2 commits March 17, 2026 11:26

Code cleanup

5c29324

Revert keys change

3e9132a

kriszyp marked this pull request as ready for review March 17, 2026 18:25

kriszyp requested a review from a team as a code owner March 17, 2026 18:25

DavidCockerill approved these changes Mar 18, 2026

View reviewed changes

dawsontoth approved these changes Mar 18, 2026

View reviewed changes

cb1kenobi reviewed Mar 18, 2026

View reviewed changes

kriszyp and others added 3 commits March 18, 2026 10:30

Make amaro be an optional dependency

ddc5ea4

Update security/jsLoader.ts

d254fa9

Co-authored-by: Chris Barber <chris@harperdb.io>

All in on amaro external package

39d1919

Ethan-Arrowood approved these changes Mar 18, 2026

View reviewed changes

Comment thread security/jsLoader.ts Outdated

Comment thread security/jsLoader.ts Outdated

Ethan-Arrowood approved these changes Mar 18, 2026

View reviewed changes

cb1kenobi reviewed Mar 18, 2026

View reviewed changes

Comment thread security/jsLoader.ts Outdated

Update security/jsLoader.ts

faaa5dc

Co-authored-by: Chris Barber <chris@harperdb.io>

cb1kenobi approved these changes Mar 18, 2026

View reviewed changes

kriszyp merged commit 940539d into main Mar 18, 2026
20 of 22 checks passed

kriszyp deleted the vm-load-typescript branch March 18, 2026 17:39

		amaro = await import('amaro').catch(() => null);
		return amaro.transformSync(source, { mode: 'strip-only' }).code;

Conversation

kriszyp commented Mar 17, 2026

Uh oh!

socket-security Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dawsontoth left a comment

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!

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!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Ethan-Arrowood left a comment

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!

cb1kenobi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

socket-security Bot commented Mar 17, 2026 •

edited

Loading