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

Add a path to Module (and expose it in Referrer) #3783

Merged
merged 5 commits into from
Apr 4, 2024

Conversation

hansl
Copy link
Contributor

@hansl hansl commented Apr 2, 2024

This allows SimpleModuleLoader (and other loaders) to resolve relative to the current file (which this commit also does).

Fixes #3782
Closes #3781

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 59.09091% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 50.20%. Comparing base (6ddc2b4) to head (063caa6).
Report is 116 commits behind head on main.

Files Patch % Lines
core/engine/src/module/loader.rs 46.66% 8 Missing ⚠️
core/engine/src/module/mod.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3783      +/-   ##
==========================================
+ Coverage   47.24%   50.20%   +2.96%     
==========================================
  Files         476      456      -20     
  Lines       46892    44800    -2092     
==========================================
+ Hits        22154    22493     +339     
+ Misses      24738    22307    -2431     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks perfect to me! Thank you.

Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Nice work!

@raskad raskad added this pull request to the merge queue Apr 3, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2024
* Add an integration test for relative imports on SimpleModuleLoader

* Add a path to Module (and expose it in Referrer)

This allows SimpleModuleLoader to resolve relative to the current
file (which this commit also does).

Fixes #3782

* cargo clippy and fmt

* prettier
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 3, 2024
@jedel1043
Copy link
Member

Probably needs a rebase, since this PR doesn't have the changes of #3772.

@hansl
Copy link
Contributor Author

hansl commented Apr 4, 2024

@jedel1043 done.

@jedel1043 jedel1043 enabled auto-merge April 4, 2024 04:48
@jedel1043 jedel1043 added this pull request to the merge queue Apr 4, 2024
Merged via the queue into boa-dev:main with commit cb5cea6 Apr 4, 2024
14 checks passed
@jedel1043
Copy link
Member

jedel1043 commented Apr 4, 2024

Forgot to check the tests, and I think this regressed some of them. Will have to investigate what's the problem, but I'm pretty sure it's the additional check added to the default loader. I don't think this needs a rollback, since it should be a fairly easy fix.

Test262 conformance changes

Test result main count PR count difference
Total 50,268 50,268 0
Passed 42,773 42,599 -174
Ignored 1,391 1,391 0
Failed 6,104 6,278 +174
Panics 18 18 0
Conformance 85.09% 84.74% -0.35%
Broken tests (174):
test/built-ins/Proxy/preventExtensions/trap-is-undefined-target-is-proxy.js (previously Passed)
test/language/expressions/dynamic-import/eval-self-once-module.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-in.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-cls-name-meth.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-cls-anon.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-fn-anon.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-gen-anon.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-cls-anon.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-cls-named.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-gen-named.js (previously Passed)
test/language/expressions/dynamic-import/reuse-namespace-object-from-import.js (previously Passed)
test/language/expressions/dynamic-import/imported-self-update.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-expr-fn-named.js (previously Passed)
test/language/expressions/dynamic-import/eval-export-dflt-cls-named.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-function-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-block-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-else-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-while-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-if-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/top-level-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-gen-return-await-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-function-return-await-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-arrow-function-await-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-function-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-block-labeled-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-arrow-function-return-await-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-do-while-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-gen-await-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-arrow-import-catch-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/catch/nested-async-function-await-eval-script-code-target.js (previously Passed)
test/language/expressions/dynamic-import/namespace/await-ns-get-nested-namespace-dflt-indirect.js (previously Passed)
test/language/expressions/dynamic-import/namespace/promise-then-ns-get-nested-namespace-dflt-indirect.js (previously Passed)
test/language/expressions/import.meta/distinct-for-each-module.js (previously Passed)
test/language/module-code/instn-resolve-err-syntax-1.js (previously Passed)
test/language/module-code/instn-iee-err-dflt-thru-star.js (previously Passed)
test/language/module-code/instn-named-err-ambiguous.js (previously Passed)
test/language/module-code/instn-iee-err-circular-as.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-fun-named.js (previously Passed)
test/language/module-code/instn-uniq-env-rec.js (previously Passed)
test/language/module-code/instn-named-err-not-found.js (previously Passed)
test/language/module-code/instn-star-id-name.js (previously Passed)
test/language/module-code/instn-named-err-not-found-as.js (previously Passed)
test/language/module-code/instn-resolve-empty-import.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-in.js (previously Passed)
test/language/module-code/instn-iee-bndng-const.js (previously Passed)
test/language/module-code/instn-named-iee-cycle.js (previously Passed)
test/language/module-code/eval-export-dflt-cls-name-meth.js (previously Passed)
test/language/module-code/instn-named-err-dflt-thru-star-as.js (previously Passed)
test/language/module-code/eval-gtbndng-indirect-update-dflt.js (previously Passed)
test/language/module-code/instn-iee-err-not-found-as.js (previously Passed)
test/language/module-code/export-expname-import-string-binding.js (previously Passed)
test/language/module-code/instn-iee-bndng-let.js (previously Passed)
test/language/module-code/export-expname-binding-string.js (previously Passed)
test/language/module-code/instn-named-bndng-cls.js (previously Passed)
test/language/module-code/instn-star-props-dflt-keep-indirect.js (previously Passed)
test/language/module-code/instn-iee-bndng-cls.js (previously Passed)
test/language/module-code/instn-iee-star-cycle.js (previously Passed)
test/language/module-code/eval-export-dflt-cls-anon.js (previously Passed)
test/language/module-code/verify-dfs.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-fun-anon.js (previously Passed)
test/language/module-code/instn-resolve-err-syntax-2.js (previously Passed)
test/language/module-code/eval-gtbndng-indirect-update.js (previously Passed)
test/language/module-code/instn-star-as-props-dflt-skip.js (previously Passed)
test/language/module-code/instn-star-iee-cycle.js (previously Passed)
test/language/module-code/instn-iee-err-not-found.js (previously Passed)
test/language/module-code/eval-gtbndng-indirect-update-as.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-fn-anon.js (previously Passed)
test/language/module-code/export-expname-from-binding-string.js (previously Passed)
test/language/module-code/instn-named-err-dflt-thru-star-dflt.js (previously Passed)
test/language/module-code/instn-star-ambiguous.js (previously Passed)
test/language/module-code/instn-resolve-order-depth.js (previously Passed)
test/language/module-code/instn-named-bndng-gen.js (previously Passed)
test/language/module-code/instn-resolve-empty-export.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-gen-anon.js (previously Passed)
test/language/module-code/export-expname-from-star-string.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-cls-anon.js (previously Passed)
test/language/module-code/instn-named-err-not-found-dflt.js (previously Passed)
test/language/module-code/instn-star-star-cycle.js (previously Passed)
test/language/module-code/instn-named-bndng-fun.js (previously Passed)
test/language/module-code/instn-same-global.js (previously Passed)
test/language/module-code/eval-rqstd-order.js (previously Passed)
test/language/module-code/instn-named-bndng-const.js (previously Passed)
test/language/module-code/instn-resolve-order-src.js (previously Passed)
test/language/module-code/instn-named-star-cycle.js (previously Passed)
test/language/module-code/export-expname-from-string.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-cls.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-cls-named.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-expr.js (previously Passed)
test/language/module-code/instn-star-props-dflt-keep-local.js (previously Passed)
test/language/module-code/instn-named-id-name.js (previously Passed)
test/language/module-code/instn-iee-trlng-comma.js (previously Passed)
test/language/module-code/instn-named-bndng-var.js (previously Passed)
test/language/module-code/instn-named-bndng-let.js (previously Passed)
test/language/module-code/instn-star-err-not-found.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-star.js (previously Passed)
test/language/module-code/export-expname-from-string-string.js (previously Passed)
test/language/module-code/instn-star-props-circular.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-named.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-gen-named.js (previously Passed)
test/language/module-code/instn-named-err-ambiguous-as.js (previously Passed)
test/language/module-code/instn-iee-bndng-var.js (previously Passed)
test/language/module-code/instn-iee-err-ambiguous-as.js (previously Passed)
test/language/module-code/instn-star-props-nrml.js (previously Passed)
test/language/module-code/instn-star-equality.js (previously Passed)
test/language/module-code/instn-once.js (previously Passed)
test/language/module-code/instn-star-props-dflt-skip.js (previously Passed)
test/language/module-code/instn-star-binding.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-gen-anon.js (previously Passed)
test/language/module-code/eval-self-once.js (previously Passed)
test/language/module-code/eval-gtbndng-indirect-trlng-comma.js (previously Passed)
test/language/module-code/instn-iee-err-ambiguous.js (previously Passed)
test/language/module-code/instn-named-bndng-trlng-comma.js (previously Passed)
test/language/module-code/instn-iee-bndng-fun.js (previously Passed)
test/language/module-code/eval-export-dflt-expr-fn-named.js (previously Passed)
test/language/module-code/instn-iee-bndng-gen.js (previously Passed)
test/language/module-code/eval-rqstd-once.js (previously Passed)
test/language/module-code/export-expname-from-star.js (previously Passed)
test/language/module-code/instn-iee-iee-cycle.js (previously Passed)
test/language/module-code/instn-iee-err-dflt-thru-star-as.js (previously Passed)
test/language/module-code/eval-rqstd-abrupt.js (previously Passed)
test/language/module-code/export-expname-binding-index.js (previously Passed)
test/language/module-code/instn-named-bndng-dflt-gen-named.js (previously Passed)
test/language/module-code/instn-iee-err-circular.js (previously Passed)
test/language/module-code/eval-export-dflt-cls-named.js (previously Passed)
test/language/module-code/export-star-as-dflt.js (previously Passed)
test/language/module-code/export-expname-from-string-binding.js (previously Passed)
test/language/module-code/top-level-await/dynamic-import-rejection.js (previously Passed)
test/language/module-code/top-level-await/module-import-resolution.js (previously Passed)
test/language/module-code/top-level-await/await-dynamic-import-resolution.js (previously Passed)
test/language/module-code/top-level-await/module-import-rejection-body.js (previously Passed)
test/language/module-code/top-level-await/dynamic-import-resolution.js (previously Passed)
test/language/module-code/top-level-await/module-import-rejection-tick.js (previously Passed)
test/language/module-code/top-level-await/module-import-rejection.js (previously Passed)
test/language/module-code/top-level-await/module-import-unwrapped.js (previously Passed)
test/language/module-code/top-level-await/module-sync-import-async-resolution-ticks.js (previously Passed)
test/language/module-code/top-level-await/module-async-import-async-resolution-ticks.js (previously Passed)
test/language/module-code/top-level-await/dfs-invariant.js (previously Passed)
test/language/module-code/top-level-await/module-self-import-async-resolution-ticks.js (previously Passed)
test/language/module-code/namespace/Symbol.toStringTag.js (previously Passed)
test/language/module-code/namespace/Symbol.iterator.js (previously Passed)
test/language/module-code/namespace/internals/get-own-property-str-found-uninit.js (previously Passed)
test/language/module-code/namespace/internals/is-extensible.js (previously Passed)
test/language/module-code/namespace/internals/has-property-sym-not-found.js (previously Passed)
test/language/module-code/namespace/internals/get-own-property-str-found-init.js (previously Passed)
test/language/module-code/namespace/internals/delete-exported-uninit.js (previously Passed)
test/language/module-code/namespace/internals/get-own-property-str-not-found.js (previously Passed)
test/language/module-code/namespace/internals/delete-non-exported.js (previously Passed)
test/language/module-code/namespace/internals/get-str-found-uninit.js (previously Passed)
test/language/module-code/namespace/internals/set-prototype-of.js (previously Passed)
test/language/module-code/namespace/internals/get-str-update.js (previously Passed)
test/language/module-code/namespace/internals/get-nested-namespace-props-nrml.js (previously Passed)
test/language/module-code/namespace/internals/own-property-keys-sort.js (previously Passed)
test/language/module-code/namespace/internals/get-str-found-init.js (previously Passed)
test/language/module-code/namespace/internals/has-property-str-not-found.js (previously Passed)
test/language/module-code/namespace/internals/object-hasOwnProperty-binding-uninit.js (previously Passed)
test/language/module-code/namespace/internals/has-property-str-found-uninit.js (previously Passed)
test/language/module-code/namespace/internals/get-sym-found.js (previously Passed)
test/language/module-code/namespace/internals/own-property-keys-binding-types.js (previously Passed)
test/language/module-code/namespace/internals/get-sym-not-found.js (previously Passed)
test/language/module-code/namespace/internals/object-keys-binding-uninit.js (previously Passed)
test/language/module-code/namespace/internals/get-str-not-found.js (previously Passed)
test/language/module-code/namespace/internals/delete-exported-init.js (previously Passed)
test/language/module-code/namespace/internals/object-propertyIsEnumerable-binding-uninit.js (previously Passed)
test/language/module-code/namespace/internals/set-prototype-of-null.js (previously Passed)
test/language/module-code/namespace/internals/prevent-extensions.js (previously Passed)
test/language/module-code/namespace/internals/set.js (previously Passed)
test/language/module-code/namespace/internals/has-property-sym-found.js (previously Passed)
test/language/module-code/namespace/internals/define-own-property.js (previously Passed)
test/language/module-code/namespace/internals/enumerate-binding-uninit.js (previously Passed)
test/language/module-code/namespace/internals/get-str-initialize.js (previously Passed)
test/language/module-code/namespace/internals/get-nested-namespace-dflt-skip.js (previously Passed)
test/language/module-code/namespace/internals/get-prototype-of.js (previously Passed)
test/language/module-code/namespace/internals/get-own-property-sym.js (previously Passed)
test/language/module-code/namespace/internals/has-property-str-found-init.js (previously Passed)

@hansl
Copy link
Contributor Author

hansl commented Apr 4, 2024

I can look into it.

@hansl hansl deleted the fix-3782 branch April 4, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relative imports in SimpleModuleLoader fail to resolve
3 participants