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

Clean up after merging webcrypto & making module globally avaliable #4278

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

olegbespalov
Copy link
Contributor

@olegbespalov olegbespalov commented Jan 24, 2025

What?

This PR does clean-ups after merge of module and also makes crypto (and other methods) globally available

Why?

This finalizes graduation of the web crypto

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)

#4188

Closes: #3154
Closes: #4031

@olegbespalov olegbespalov self-assigned this Jan 24, 2025
@olegbespalov olegbespalov requested a review from a team as a code owner January 24, 2025 11:13
@olegbespalov olegbespalov requested review from mstoykov and joanlopez and removed request for a team January 24, 2025 11:13
joanlopez
joanlopez previously approved these changes Jan 24, 2025
@olegbespalov
Copy link
Contributor Author

@joanlopez for the record, I've just did rebase and also adjusted texting on when the experimental import will be removed The k6/experimental/webcrypto will be removed in k6 v1.1.0

joanlopez
joanlopez previously approved these changes Feb 19, 2025
@mstoykov
Copy link
Contributor

Sorry for the slow reply, but didn't have time before to finish my proof of concept for k6/timers not being done in the same way.

I did also technically went for a different way of implementing #4563 originally, but ended up being so convoluted that it made no sense. And in practice the only reason why it needed some of the stuff is so that import { setTimeout } from "k6/timers" would've been nice to have the same value as globalThis.setTimeout.

Arguably k6/timers might not need to exist, but k6/webcrypto definitely seems like it doesn't need to at all.

all that to say @olegbespalov please look at #4563 and let see if we agree on the approach of moving to js/tc55 or something like that and not actually making modules out of stuff.

@olegbespalov
Copy link
Contributor Author

Hey @mstoykov, I've partially adopted the approach that you've used in #4563 (without actual extraction of the crypto logic) as part of this PR. So please give this PR review.

@olegbespalov olegbespalov merged commit 79ba29f into master Feb 20, 2025
29 checks passed
@olegbespalov olegbespalov deleted the feat/webcrypto-global branch February 20, 2025 14:19
@olegbespalov olegbespalov added the documentation-needed A PR which will need a separate PR for documentation label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: webcrypto documentation-needed A PR which will need a separate PR for documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make webcrypto avaliable globaly as crypto Making the k6/experimental/webcrypto module part of the k6 core
3 participants