src: port initializeImportMeta to native

[aduh95]

Supersedes #57003.

We should probably avoid translating the URL to a path twice when the user needs both dirname and filename, but I'm not sure if there's an elegant way to do it without crossing the C++/JS bundary.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[codecov]

Codecov Report

Attention: Patch coverage is 77.27273% with 15 lines in your changes missing coverage. Please review.

Project coverage is 90.24%. Comparing base (6b0af17) to head (d5b268f).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_modules.cc	74.57%	4 Missing and 11 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57286      +/-   ##
==========================================
- Coverage   90.24%   90.24%   -0.01%     
==========================================
  Files         630      630              
  Lines      184908   184970      +62     
  Branches    36181    36203      +22     
==========================================
+ Hits       166874   166927      +53     
- Misses      11061    11073      +12     
+ Partials     6973     6970       -3     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/initialize_import_meta.js	100.00% <100.00%> (ø)
src/node_modules.cc	78.42% <74.57%> (-0.59%)	⬇️

... and 35 files with indirect coverage changes

[JakobJingleheimer]

I'm not great with cpp, but it looks fine AFAICT, and conceptually checks out 🙂

[JakobJingleheimer]

Suggested change

	meta, url, StringPrototypeStartsWith(url, 'file:'),
	meta,
	url,
	StringPrototypeStartsWith(url, 'file:'),

[JakobJingleheimer]

I think there's a typo here: and. Retrieve

[aduh95]

Ah, that was taken from

node/src/node_util.cc

Line 360 in e03af77

// returns a nullptr and. Retrieve the creation context via `this` object and

I've opened #57291 to fix it

[geeksilva97]

We could use env properties.

Suggested change

	target
	->SetLazyDataProperty(context,
	FIXED_ONE_BYTE_STRING(isolate, "dirname"),
	InitImportMetaLazyGetter,
	args[1])
	.Check();
	target
	->SetLazyDataProperty(context,
	FIXED_ONE_BYTE_STRING(isolate, "filename"),
	InitImportMetaLazyGetter,
	args[1])
	.Check();
	target
	->SetLazyDataProperty(context,
	env->dirname_string(),
	InitImportMetaLazyGetter,
	args[1])
	.Check();
	target
	->SetLazyDataProperty(context,
	env->filename_string(),
	InitImportMetaLazyGetter,
	args[1])
	.Check();

Would need to retrieve env. Maybe with Environment* env = Environment::GetCurrent(args);.

And would need to add missing entries to https://github.com/nodejs/node/blob/main/src/env_properties.h

[aduh95]

What would be the reasoning for doing that?

[geeksilva97]

I'm too new to the codebase to have a deep explanation.

Here are my newbie reasons: it makes the code cleaner and it has been used in the C++ land (at least in my recent C++ PRs, more experienced people suggested I do this)

[jasnell]

Well, env->filename_string() already exists so it would make sense to use it rather than creating a new copy. If we're using that then we may as well as one for dirname... keeping in mind, however, that there is already one for __dirname.

[geeksilva97]

Like the previous comment

Suggested change

	target->Set(context, FIXED_ONE_BYTE_STRING(isolate, "resolve"), args[3])
	.Check();
	}
	target->Set(context, FIXED_ONE_BYTE_STRING(isolate, "url"), args[1]).Check();
	target->Set(context, env->resolve_string(), args[3])
	.Check();
	}
	target->Set(context, env->url_string(), args[1]).Check();

[jsumners-nr]

I think the original implementation is much easier to understand and maintain.

[aduh95]

I think the original implementation is much easier to understand and maintain.

Do you have any suggestions on how to improve it?