Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement import.meta.resolve() #3873

Merged
merged 3 commits into from
Aug 1, 2024
Merged

Implement import.meta.resolve() #3873

merged 3 commits into from
Aug 1, 2024

Conversation

mstoykov
Copy link
Contributor

What?

This implements and tests import.meta.resolve() a way to get the path to a module the same way ESM and/or require will do.

This doesn't read or load the file in question from the file.

This also showed that we currently always parse code as ECMAScript modules. Even when the parsed/compiled file was to be wrapped as CommonJS.

This likely had no other side effects as all other ESM specific syntax is also either not implemented in k6 or forces CommonJS wrapping disabled.

Why?

This lets people use open, require , experimental/fs.open, etc. and have a way to always be relative to the module it is written in.

The fixes to compiler were as otherwise CommonJS modules don't error on import.meta and doesn't just isn't supported in the standard.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

#3856

This implements and tests `import.meta.resolve()` a way to get the path
to a module the same way ESM and/or `require` will do.

This doesn't read or load the file in question from the file.

This also showed that we currently always parse code as ECMAScript
modules. Even when the parsed/compiled file was to be wrapped as
CommonJS.

This likely had no other side effects as all other ESM specific syntax
is also either not implemented in k6 or forces CommonJS wrapping
disabled.

Closes #3856
@mstoykov mstoykov added feature documentation-needed A PR which will need a separate PR for documentation labels Jul 26, 2024
@mstoykov mstoykov requested a review from a team as a code owner July 26, 2024 14:00
@mstoykov mstoykov requested review from codebien and joanlopez and removed request for a team July 26, 2024 14:00
codebien
codebien previously approved these changes Jul 30, 2024
js/bundle.go Outdated
@@ -451,6 +451,16 @@ func (b *Bundle) setInitGlobals(rt *sobek.Runtime, vu *moduleVUImpl, modSys *mod
}
warnAboutModuleMixing("module")
warnAboutModuleMixing("exports")

rt.SetFinalImportMeta(func(o *sobek.Object, mr sobek.ModuleRecord) {
_ = o.Set("resolve", func(specifier string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we do something with the error in case there's any? If it isn't supposed to happen, at all, I'm generally in favor of panicking.

joanlopez
joanlopez previously approved these changes Jul 30, 2024
Copy link
Contributor

@joanlopez joanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of minor comments, but it looks generally good!

Co-authored-by: Joan López de la Franca Beltran <5459617+joanlopez@users.noreply.github.com>
@mstoykov mstoykov dismissed stale reviews from joanlopez and codebien via add1fb4 July 30, 2024 12:13
@joanlopez joanlopez added this to the v0.53.0 milestone Aug 1, 2024
@joanlopez joanlopez merged commit 913b280 into master Aug 1, 2024
23 checks passed
@joanlopez joanlopez deleted the importeMetaResolve branch August 1, 2024 09:09
@mstoykov mstoykov linked an issue Aug 12, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-needed A PR which will need a separate PR for documentation feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement import.meta.resolve
3 participants