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

Make the point that compatibility-mode=base is a lot less useful now #794

Closed
mstoykov opened this issue Sep 8, 2022 · 4 comments · Fixed by #1489
Closed

Make the point that compatibility-mode=base is a lot less useful now #794

mstoykov opened this issue Sep 8, 2022 · 4 comments · Fixed by #1489
Labels
Area: OSS Content Improvements or additions to community/oss documentation

Comments

@mstoykov
Copy link
Contributor

mstoykov commented Sep 8, 2022

History

At the time --compatibility-mode= was added, with possible values of base and extented, k6 was using both babel and core.js v2 internally to "extend" what js k6 could run.

Since then we have completely dropped core.js which was arguably the biggest contributter ro making compatibilty-mode=base perform a lot better. It both dropped 2mb+ of memory per VU, but also removed the need for each VU to run 84kb of minified js code. That was taking considerably enough time, that when we removed it suddenly k6 VUS were starting really faster.

Since then we have also dropped almost everything else that babel is used except for ecmascript module support (import/export syntax ++). This is likely to change soon ™️ see grafana/k6#2563

Problem

But users keep going for using --compatibility-mode=base as a way to optimize their scripts, which usually requires them to:

  1. setup babel/webpack
  2. run additional commands
  3. or write (at least) commonjs by hand

And at the end they usually get nothing in return except for a harder setup :(.

Given this I think it's good idea to start recommending people to not make their scripts --compatibility-mode=base if they want more performance as that will just not work.

Caveats

  1. --compatibilty-mode=base will still be useful until we completely remove babel if the users have particuarly big scripts and them going through our babel is time consuming. This usually happens with scripts that have combined length of over 10k lines[citation needed]. Given also that we are likely to have this if not in v0.41.0 then in v0.42.0, it might still be worth it to start telling people to not go through the trouble for this.
  2. Some users want to use code written for nodejs and running it through webpack/babel is what makes it (sometimes) work with k6. This also applies for users using typescript. This is why https://github.com/grafana/k6-template-es6 and https://github.com/grafana/k6-template-typescript/ exists. But they both should probably be updated to not go to commonjs but instead still use ESM after support for it is added in k6.

Proposed solution:

  1. Deprecate most of the information on https://k6.io/docs/using-k6/javascript-compatibility-mode/ and wherever else it's recommended for performance reasons
  2. Try to be more explicit on what --compatibility-mode=extented actually still extends - this is now pretty short, it adds global as alias to globaThis and adds support for ESM as it runs the code through babel (This is as of v0.40.0 which did add class support).
  3. (not for the k6-docs repo) Update the template repositories to not transpile stuff k6 already supports.
@mstoykov mstoykov added Area: OSS Content Improvements or additions to community/oss documentation Priority: Medium Type: Bug Something doesn't work how it should labels Sep 8, 2022
@MattDodsonEnglish
Copy link
Contributor

Deprecate most of the information on https://k6.io/docs/using-k6/javascript-compatibility-mode/ and wherever else it's recommended for performance reason

One thing to note, this page is already hidden from the sidebar. Maybe we can just delete it? 🙂 What would the consequence be?

@mstoykov
Copy link
Contributor Author

It is still a feature of k6 and you can enable it ... I am also pretty certain it is linked from a lot places.

So we should probably update it to tell the users that it is no longer needed more than anything else.

@MattDodsonEnglish
Copy link
Contributor

But what if we just:

  1. Put this section in a collapsible: https://k6.io/docs/using-k6/k6-options/reference/#compatibility-mode
  2. Added an admonition in this collapsible with all the necessary deprecation
  3. Redirected docs/using-k6/javascript-compatibility-mode/ to that link, anchored at that specific slug.

Do you see any problem with this? The only possible issues I foresee:

  • Some essential info from the doc doesn't make it to the admonition (though the commit history will be forever 🙂 )
  • I'm never tested redirecting to an in-page slug (but that must be possible!).

@na--
Copy link
Member

na-- commented Sep 15, 2022

I don't think we should cram this in the options section, that seems like it will make things worse 🤔 We don't want to hide --compatibility-mode, we want to explain it better.

In fact, we should probably have a new "JavaScript compatibility" docs page with:

  • information about how k6 is not node.js or a browser, i.e. basically move big parts of this document there and link to them
  • information about goja and babel and which parts of the ECMAScript specs k6 supports
  • information about --compatibility-mode, why it was created and why it's no longer very useful (and link to that section from the options and redirect to it from the current page)
  • front-and-center information about JS modules and import / require and how you shouldn't mix both if you want your scripts to work in the future
  • other useful information for users that might not be super familiar with JS, e.g. links to MDN? also links to jslib.k6.io and other useful resources like the webpack template

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OSS Content Improvements or additions to community/oss documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants