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

Fix lazy state #643

Merged
merged 2 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions integration-tests/worker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# @openfn/integration-tests-worker

## 1.0.38

### Patch Changes

- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]

## 1.0.37

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/worker/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@openfn/integration-tests-worker",
"private": true,
"version": "1.0.37",
"version": "1.0.38",
"description": "Lightning WOrker integration tests",
"author": "Open Function Group <[email protected]>",
"license": "ISC",
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# @openfn/cli

## 1.1.4

### Patch Changes

- Updated dependencies
- @openfn/[email protected]

## 1.1.3

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/cli",
"version": "1.1.3",
"version": "1.1.4",
"description": "CLI devtools for the openfn toolchain.",
"engines": {
"node": ">=18",
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# @openfn/compiler

## 0.1.1

### Patch Changes

- Ignore lazy state operator ($) for locally declared variables

## 0.1.0

### Minor Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/compiler",
"version": "0.1.0",
"version": "0.1.1",
"description": "Compiler and language tooling for openfn jobs.",
"author": "Open Function Group <[email protected]>",
"license": "ISC",
Expand Down
18 changes: 13 additions & 5 deletions packages/compiler/src/transforms/lazy-state.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
/*
* Convert $.a.b.c references into (state) => state.a.b.c
* Should this only run at top level?
* Ideally it would run on all arguments to operations - but we probably don't really know what an operation is
* So for now, first pass, it's only top level.
* (alternatively I guess it just dumbly converts everything and if it breaks, it breaks)
*
* Converts all $.a.b chains unless:
* - $ was assigned previously in that scope
*
* TODO (maybe):
* - only convert $-expressions which are arguments to operations (needs type defs)
Expand All @@ -23,6 +22,15 @@ function visitor(path: NodePath<namedTypes.MemberExpression>) {
let firstIdentifer = first as namedTypes.Identifier;

if (first && firstIdentifer.name === "$") {
// But if a $ declared a parent scope, ignore it
let scope = path.scope;
while (scope) {
if (!scope.isGlobal && scope.declares('$')) {
return false;
}
scope = scope.parent;
}

// rename $ to state
firstIdentifer.name = "state";

Expand All @@ -36,7 +44,7 @@ function visitor(path: NodePath<namedTypes.MemberExpression>) {
}

// Stop parsing this member expression
return;
return false;
}

export default {
Expand Down
41 changes: 30 additions & 11 deletions packages/compiler/test/transforms/lazy-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ test('convert a simple dollar reference', (t) => {

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)
t.log(code)

t.is(code, 'get(state => state.data)')
})
Expand All @@ -22,7 +21,6 @@ test('convert a chained dollar reference', (t) => {

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)
t.log(code)

t.is(code, 'get(state => state.a.b.c.d)')
})
Expand All @@ -32,7 +30,6 @@ test('ignore a regular chain reference', (t) => {

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)
t.log(code)

t.is(code, 'get(a.b.c.d)')
})
Expand All @@ -42,7 +39,6 @@ test('ignore a string', (t) => {

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)
t.log(code)

t.is(code, 'get("$.a.b")')
})
Expand All @@ -55,7 +51,6 @@ test('convert a nested dollar reference', (t) => {

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)
t.log(code)

// syntax starts getting a but picky at this level,
// better to do ast tests
Expand All @@ -64,13 +59,37 @@ test('convert a nested dollar reference', (t) => {
})`)
})

// TODO does our compiler not support optional chaining??
test.skip('convert an optional chained simple dollar reference', (t) => {
test('do not convert a $ var (param)', (t) => {
const src = `fn(($) => {
return $.a.b;
})`
const ast = parse(src);

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)

t.is(code, src)
})

test('do not convert a $ var (const)', (t) => {
const src = `fn((s) => {
const $ = 10;
s.data = $.a.b
return s;
})`
const ast = parse(src);

const transformed = transform(ast, [visitors]);
const { code } = print(transformed)

t.is(code, src)
})

test('convert an optional chained simple dollar reference', (t) => {
const ast = parse('get($.a?.b.c.d)');

// const transformed = transform(ast, [visitors]);
// const { code } = print(transformed)
// t.log(code)
const transformed = transform(ast, [visitors]);
const { code } = print(transformed)

// t.is(code, 'get(state => state.a?.b.c.d)')
t.is(code, 'get(state => state.a?.b.c.d)')
})
7 changes: 7 additions & 0 deletions packages/engine-multi/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# engine-multi

## 1.1.3

### Patch Changes

- Updated dependencies
- @openfn/[email protected]

## 1.1.2

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/engine-multi/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/engine-multi",
"version": "1.1.2",
"version": "1.1.3",
"description": "Multi-process runtime engine",
"main": "dist/index.js",
"type": "module",
Expand Down
6 changes: 6 additions & 0 deletions packages/lightning-mock/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# @openfn/lightning-mock

## 2.0.3

### Patch Changes

- @openfn/[email protected]

## 2.0.2

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/lightning-mock/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/lightning-mock",
"version": "2.0.2",
"version": "2.0.3",
"private": true,
"description": "A mock Lightning server",
"main": "dist/index.js",
Expand Down
6 changes: 6 additions & 0 deletions packages/ws-worker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# ws-worker

## 1.1.3

### Patch Changes

- @openfn/[email protected]

## 1.1.2

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/ws-worker/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/ws-worker",
"version": "1.1.2",
"version": "1.1.3",
"description": "A Websocket Worker to connect Lightning to a Runtime Engine",
"main": "dist/index.js",
"type": "module",
Expand Down