-
Notifications
You must be signed in to change notification settings - Fork 130
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
feat(core) New tracer API proposal #2430
base: main
Are you sure you want to change the base?
Changes from all commits
ca94fc5
dfffa6c
c16a808
0887285
7a1ec97
5c5826b
55483fe
cb5845f
46ad3d5
e7b7798
64d7083
4afa9ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@envelop/core": patch | ||
--- | ||
dependencies updates: | ||
- Added dependency [`@envelop/instruments@workspace:^` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/workspace:^) (to `dependencies`) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'@envelop/types': minor | ||
'@envelop/core': minor | ||
'@envelop/instruments': major | ||
--- | ||
|
||
Add new Tracer API |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,3 +115,5 @@ website/public/sitemap.xml | |
.tool-versions | ||
|
||
.mise.toml | ||
.helix/config.toml | ||
.helix/languages.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
import { envelop, Instruments, useEngine } from '@envelop/core'; | ||
|
||
describe('instruments', () => { | ||
it('should instrument all graphql phases', async () => { | ||
const result: string[] = []; | ||
const instrument: Instruments<any> = { | ||
init: (_, w) => { | ||
result.push('pre-init'); | ||
expect(w()).toBeUndefined(); | ||
result.push('post-init'); | ||
return 'instrument'; | ||
}, | ||
parse: (_, w) => { | ||
result.push('pre-parse'); | ||
expect(w()).toBeUndefined(); | ||
result.push('post-parse'); | ||
return 'instrument'; | ||
}, | ||
validate: (_, w) => { | ||
result.push('pre-validate'); | ||
expect(w()).toBeUndefined(); | ||
result.push('post-validate'); | ||
return 'instrument'; | ||
}, | ||
context: (_, w) => { | ||
result.push('pre-context'); | ||
expect(w()).toBeUndefined(); | ||
result.push('post-context'); | ||
return 'instrument'; | ||
}, | ||
// @ts-expect-error Returning something other than undefined should not be allowed | ||
execute: async (_, w) => { | ||
result.push('pre-execute'); | ||
expect(await w()).toBeUndefined(); | ||
result.push('post-execute'); | ||
return 'instrument'; | ||
}, | ||
// @ts-expect-error Returning something other than undefined shoould not be allowed | ||
subscribe: async (_, w) => { | ||
result.push('pre-subscribe'); | ||
expect(await w()).toBeUndefined(); | ||
result.push('post-subscribe'); | ||
return 'instrument'; | ||
}, | ||
}; | ||
|
||
const getEnveloped = envelop({ | ||
plugins: [ | ||
useEngine({ | ||
execute: () => { | ||
result.push('execute'); | ||
return new Promise(r => setTimeout(() => r('test'), 10)); | ||
}, | ||
subscribe: () => { | ||
result.push('subscribe'); | ||
return new Promise(r => setTimeout(() => r('test'), 10)); | ||
}, | ||
parse: () => { | ||
result.push('parse'); | ||
return { test: 'foo' }; | ||
}, | ||
validate: () => { | ||
result.push('validate'); | ||
return 'test'; | ||
}, | ||
}), | ||
{ instruments: instrument }, | ||
], | ||
}); | ||
|
||
const gql = getEnveloped({ test: 'foo' }); | ||
expect(gql.parse('')).toEqual({ test: 'foo' }); | ||
expect(gql.validate({}, {})).toEqual('test'); | ||
expect(gql.contextFactory()).toEqual({ test: 'foo' }); | ||
expect(await gql.execute({ document: {}, schema: {} })).toEqual('test'); | ||
expect(await gql.subscribe({ document: {}, schema: {} })).toEqual('test'); | ||
|
||
expect(result).toEqual([ | ||
'pre-init', | ||
'post-init', | ||
'pre-parse', | ||
'parse', | ||
'post-parse', | ||
'pre-validate', | ||
'validate', | ||
'post-validate', | ||
'pre-context', | ||
'post-context', | ||
'pre-execute', | ||
'execute', | ||
'post-execute', | ||
'pre-subscribe', | ||
'subscribe', | ||
'post-subscribe', | ||
]); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
## `@envelop/instruments` | ||
|
||
This package contains uitility functions and types to ease the use of instruments accross Envelop, | ||
Yoga, wathwg-node and Hive Gateway plugins. | ||
|
||
### `getInstrumentsAndPlugins(plugins: Plugin[]): { pluginInstruments: Instruments[], plugins: Plugin[] }` | ||
|
||
This function extracts the instruments from the plugins and returns both the extracted instruments | ||
and the plugins without their `instruments` field. | ||
|
||
This is usefull when you want to customize the execution order of the instruments. | ||
|
||
```ts | ||
import { getInstrumentsAndPlugins } from '@envelop/instruments' | ||
|
||
const { pluginInstruments, plugins } = getInstrumentsAndPlugins([ | ||
// put you plugin list here. This list can contain plugins with and without instruments. | ||
]) | ||
``` | ||
|
||
## `composeInstruments(instruments: Instruments[]): Instruments` | ||
|
||
This function composes all the instruments into one. The instruments will be called in the same | ||
order than they are in the array (first is outter most call, last is inner most). | ||
|
||
This can be used in conjonction with `getInstrumentsAndPlugins` function to customize the order of | ||
execution of the instruments if the default one doesn't suites your needs. | ||
|
||
```ts | ||
import { getInstrumentsAndPlugins } from '@envelop/instruments' | ||
|
||
const { pluginInstruments, plugins } = getInstrumentsAndPlugins([ | ||
// put you plugin list here. This list can contain plugins with and without instruments. | ||
]) | ||
|
||
const instruments = composeInstruments(pluginInstruments) | ||
|
||
const getEnveloped = envelop({ | ||
plugins: [...plugins, { instruments }] | ||
}) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
{ | ||
"name": "@envelop/instruments", | ||
"version": "0.0.0", | ||
"type": "module", | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/n1ru4l/envelop.git", | ||
"directory": "packages/instruments" | ||
}, | ||
"author": "Valentin Cocaud <[email protected]>", | ||
"license": "MIT", | ||
"engines": { | ||
"node": ">=18.0.0" | ||
}, | ||
"main": "dist/cjs/index.js", | ||
"module": "dist/esm/index.js", | ||
"exports": { | ||
".": { | ||
"require": { | ||
"types": "./dist/typings/index.d.cts", | ||
"default": "./dist/cjs/index.js" | ||
}, | ||
"import": { | ||
"types": "./dist/typings/index.d.ts", | ||
"default": "./dist/esm/index.js" | ||
}, | ||
"default": { | ||
"types": "./dist/typings/index.d.ts", | ||
"default": "./dist/esm/index.js" | ||
} | ||
}, | ||
"./*": { | ||
"require": { | ||
"types": "./dist/typings/*.d.cts", | ||
"default": "./dist/cjs/*.js" | ||
}, | ||
"import": { | ||
"types": "./dist/typings/*.d.ts", | ||
"default": "./dist/esm/*.js" | ||
}, | ||
"default": { | ||
"types": "./dist/typings/*.d.ts", | ||
"default": "./dist/esm/*.js" | ||
} | ||
}, | ||
"./package.json": "./package.json" | ||
}, | ||
"typings": "dist/typings/index.d.ts", | ||
"dependencies": { | ||
"@whatwg-node/promise-helpers": "^1.2.1", | ||
"tslib": "^2.5.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isnt this usual a dev and peer dependency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tslib is imported and used by the JavaScript files compiled by tsc when you have "importHelpers" enabled to dedupe helper methods in the compiled codd There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know to be honest, it is like this in other packages so I just copied the same setup ^^' |
||
}, | ||
"devDependencies": { | ||
"typescript": "5.7.3" | ||
}, | ||
"publishConfig": { | ||
"directory": "dist", | ||
"access": "public" | ||
}, | ||
"sideEffects": false, | ||
"buildOptions": { | ||
"input": "./src/index.ts" | ||
}, | ||
"typescript": { | ||
"definition": "dist/typings/index.d.ts" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
// eslint-disable-next-line import/export | ||
export * from './instruments.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the point of this then is to be able to extract the instruments from an existing list of plugins, and then tack them on the end of the plugin list?
This is an interesting workaround and could work. An alternative approach might be to set a priority number on plugins, so that regardless of the order they are defined in, they are executed based on priority. We could then allocate certain ranges for certain things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's closed to the idea yes.
We already have a defined order of plugins execution : It's the order they are defined in the array.
The problem is that, in the case of "wrapping phases", there is often the need to have a different order for "wrapping" and for normal hooks.
So the point of those utils are to extract instruments from plugins (and remove them), so that you can compose them is the order you want. Since you know there is no other instruments, you can put it in the end of the plugin, or at the beginning, it doesn't matter :-)