Switch to our fork of goja - sobek

[mstoykov]

What?

Move to sobek from goja - currently only aliasing types over from goja.

Why?

This is part of #3772 and #3773.

The PR also doesn't try to fix every other reference of goja - a bunch of them are docs that will break at this point, and will likely need to be updated over time. Likely after sobek is an actual fork of goja.

This is marked as breaking change mostly because I do expect it will still break something, even if not for most users.

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)

[mstoykov]

This unfortunately becomes 121 symbols and also both cases hit the "is interface nil?" trap so ... yeah. This is the best I can do without having to add a lot mroe code.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 54.24354% with 124 lines in your changes missing coverage. Please review.

Project coverage is 70.86%. Comparing base (f57dc2f) to head (20b04a4).

❗ Current head 20b04a4 differs from pull request most recent head db3fb66

Please upload reports for the commit db3fb66 to get more accurate results.

Files	Patch %	Lines
...odules/k6/experimental/streams/readable_streams.go	0.00%	29 Missing ⚠️
js/modules/k6/html/elements.go	0.00%	15 Missing ⚠️
js/modules/k6/experimental/streams/module.go	0.00%	12 Missing ⚠️
js/modules/k6/html/element.go	15.38%	11 Missing ⚠️
js/modules/k6/experimental/streams/goja.go	0.00%	8 Missing ⚠️
...ntal/streams/readable_stream_default_controller.go	0.00%	6 Missing ⚠️
js/runner.go	73.68%	4 Missing and 1 partial ⚠️
js/modules/k6/html/elements_gen.go	42.85%	4 Missing ⚠️
js/modules/k6/html/html.go	71.42%	4 Missing ⚠️
js/modules/k6/http/response.go	63.63%	1 Missing and 3 partials ⚠️
... and 14 more

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3775   +/-   ##
=======================================
  Coverage   70.86%   70.86%           
=======================================
  Files         291      291           
  Lines       21226    21227    +1     
=======================================
+ Hits        15042    15043    +1     
  Misses       5219     5219           
  Partials      965      965           

Flag	Coverage Δ
ubuntu	70.80% <54.24%> (-0.02%)	⬇️
windows	70.72% <54.24%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[codebien]

Am I getting it right? Isn't anything else than s/goja/sobek/, right?

[mstoykov]

Technically it is ... as I am leaving the parser and ast packages from goja in js/compiler. Otherwise it needs a lot more aliases, which won't matter for any other codebase - no extension ever uses those.

And also I replace goja. with sobek. as to not break a bunch of comments referncing issues and docs for goja.

Both of those will be updated in the future - one of them when we move actually to a fork, the others when we have time as part of cleaning. But I didn't want to figure out all the cases for each comment individually.

[joanlopez]

All the replacements looks good, and I have also confirmed that the only remaining uses of github.com/dop251/goja are for goja/parser and goja/file, expected as described in the issue.

So, I guess I could say it looks good, as far as I can tell 😄

[joanlopez]

Hey! While working on another PR/branch, I realized that for ongoing changes, it's pretty easy to forget about goja imports, as we'll keep the dependency for a while.

Do you think worths adding a temporary linter that forbids those imports?
cc/ @mstoykov