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

Remove Sobek from Execution Context Eval #1182

Closed
8 tasks done
Tracked by #1138
ankur22 opened this issue Jan 22, 2024 · 1 comment
Closed
8 tasks done
Tracked by #1138

Remove Sobek from Execution Context Eval #1182

ankur22 opened this issue Jan 22, 2024 · 1 comment
Assignees
Labels
bug Something isn't working refactor

Comments

@ankur22
Copy link
Collaborator

ankur22 commented Jan 22, 2024

What

ExecutionContext.Eval is at the heart of the module. Removing its Goja dependency will tremendously detach goja from most of the business logic. executionContext.eval takes a slice argument, which can be of any type, including goja.Value. Find a way to enforce a change where the slice must be anything BUT a goja.Value.

Why

  • Improved stability: Enable avoiding working with the non-thread safe Goja from multiple goroutines.
  • Productivity: Business logic can use native Go constructs. Code is simpler and easy to reason about.
  • Testability: Testing the business logic becomes straightforward.

How

  • Remove goja from Eval and all related methods.
  • Ensure that none of the methods in the module passes goja values to Eval and friends.
  • Add assertions and keep them there for a few cycles.

Related

#1162

Tasks

  1. 2 of 2
    bug refactor
    inancgumus
  2. async dx refactor tests
    inancgumus
  3. bug tests
    inancgumus
  4. bug refactor tests
    inancgumus
  5. 3 of 3
    bug
    ankur22
@ankur22 ankur22 added the bug Something isn't working label Jan 22, 2024
@ankur22 ankur22 changed the title Pass slice of any (non-goja.Values) instead of slice of goja.Values into EexecutionContext.eval Pass slice of any (non-goja.Values) instead of slice of goja.Values into ExecutionContext.eval Jan 23, 2024
@inancgumus inancgumus self-assigned this Jan 24, 2024
@inancgumus inancgumus changed the title Pass slice of any (non-goja.Values) instead of slice of goja.Values into ExecutionContext.eval Remove Goja from Execution Context Eval Jan 24, 2024
@inancgumus inancgumus reopened this Feb 2, 2024
@inancgumus
Copy link
Member

Wrapping up this one 🎉

@inancgumus inancgumus changed the title Remove Goja from Execution Context Eval Remove Sobek from Execution Context Eval Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor
Projects
None yet
Development

No branches or pull requests

2 participants