Skip to content

Commit

Permalink
Don't return NodeJS.Timer from setTimeout and setInterval (#622)
Browse files Browse the repository at this point in the history
Co-authored-by: Kiko Beats <josefrancisco.verdu@gmail.com>
  • Loading branch information
smaeda-ks and Kikobeats committed Oct 2, 2023
1 parent 20424ec commit 21fa983
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 7 deletions.
7 changes: 7 additions & 0 deletions .changeset/slimy-hairs-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@edge-runtime/ponyfill": patch
"@edge-runtime/primitives": patch
"@edge-runtime/vm": patch
---

Don't return `NodeJS.Timer` from `setTimeout` and `setInterval`
2 changes: 2 additions & 0 deletions packages/ponyfill/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ function edge() {
ReadableStreamDefaultReader,
Request,
Response,
setInterval,
setTimeout,
structuredClone,
SubtleCrypto,
TextDecoder,
Expand Down
12 changes: 12 additions & 0 deletions packages/primitives/src/primitives/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,18 @@ export function load(scopedContext = {}) {
})
assign(context, { console: consoleImpl.console })

/** @type {import('../../type-definitions/timer')} */
const timeoutImpl = requireWithFakeGlobalScope({
context,
id: 'timeout.js',
sourceCode: injectSourceCode('./timer.js'),
scopedContext,
})
assign(context, {
setTimeout: timeoutImpl.setTimeout,
setInterval: timeoutImpl.setInterval,
})

/** @type {import('../../type-definitions/events')} */
const eventsImpl = requireWithFakeGlobalScope({
context,
Expand Down
10 changes: 10 additions & 0 deletions packages/primitives/src/primitives/timer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const timeoutProxy = new Proxy(setTimeout, {
apply: (target, thisArg, args) => {
const timeout = Reflect.apply(target, thisArg, args)
// Returns integer value of timeout ID
return timeout[Symbol.toPrimitive]()
},
})

export { timeoutProxy as setTimeout }
export { timeoutProxy as setInterval }
1 change: 1 addition & 0 deletions packages/primitives/type-definitions/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export * from './streams'
export * from './text-encoding-streams'
export * from './structured-clone'
export * from './url'
export * from './timer'
5 changes: 5 additions & 0 deletions packages/primitives/type-definitions/timer.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
declare const _setTimeout: typeof Number
declare const _setInterval: typeof Number

export { _setTimeout as setTimeout }
export { _setInterval as setInterval }
18 changes: 11 additions & 7 deletions packages/vm/src/edge-vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function patchInstanceOf(item: string, ctx: any) {
}
})
`,
ctx
ctx,
)
}

Expand All @@ -155,9 +155,9 @@ function registerUnhandledRejectionHandlers(handlers: RejectionHandler[]) {
'unhandledRejection',
function invokeRejectionHandlers(reason, promise) {
unhandledRejectionHandlers.forEach((handler) =>
handler({ reason, promise })
handler({ reason, promise }),
)
}
},
)
}
unhandledRejectionHandlers = handlers
Expand Down Expand Up @@ -294,6 +294,8 @@ export type EdgeContext = VMContext & {
ReadableStreamDefaultReader: typeof EdgePrimitives.ReadableStreamDefaultReader
Request: typeof EdgePrimitives.Request
Response: typeof EdgePrimitives.Response
setTimeout: typeof EdgePrimitives.setTimeout
setInterval: typeof EdgePrimitives.setInterval
structuredClone: typeof EdgePrimitives.structuredClone
SubtleCrypto: typeof EdgePrimitives.SubtleCrypto
TextDecoder: typeof EdgePrimitives.TextDecoder
Expand All @@ -315,8 +317,6 @@ function addPrimitives(context: VMContext) {
defineProperty(context, 'Symbol', { value: Symbol })
defineProperty(context, 'clearInterval', { value: clearInterval })
defineProperty(context, 'clearTimeout', { value: clearTimeout })
defineProperty(context, 'setInterval', { value: setInterval })
defineProperty(context, 'setTimeout', { value: setTimeout })
defineProperty(context, 'queueMicrotask', { value: queueMicrotask })
defineProperty(context, 'EdgeRuntime', { value: 'edge-runtime' })

Expand Down Expand Up @@ -383,6 +383,10 @@ function addPrimitives(context: VMContext) {

// Console
'console',

// Timers
'setTimeout',
'setInterval',
],
})

Expand All @@ -404,7 +408,7 @@ function defineProperties(
exports: Record<string, any>
enumerable?: string[]
nonenumerable?: string[]
}
},
) {
for (const property of options.enumerable ?? []) {
if (!options.exports[property]) {
Expand Down Expand Up @@ -433,7 +437,7 @@ function defineProperties(
* implemented in the provided context.
*/
function getTransferablePrimitivesFromContext(
context: Context
context: Context,
): Record<(typeof transferableConstructors)[number], unknown> {
const keys = transferableConstructors.join(',')
const stringifedObject = `({${keys}})`
Expand Down
18 changes: 18 additions & 0 deletions packages/vm/tests/edge-runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -622,3 +622,21 @@ describe('Event handlers', () => {
expect((runtime as any).__rejectionHandlers).toBeUndefined()
})
})

describe('`Timers`', () => {
it.each(['setTimeout', 'setInterval'])(
'%s function should return integer',
async (f) => {
const runtime = new EdgeVM()
expect(() => {
runtime.evaluate(`
const timer = ${f}(() => {}, 1000);
timer.unref();
`)
}).toThrow({
name: 'Error',
message: `timer.unref is not a function`,
})
},
)
})

1 comment on commit 21fa983

@vercel
Copy link

@vercel vercel bot commented on 21fa983 Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

edge-runtime – ./

edge-runtime.vercel.app
edge-runtime.vercel.sh
edge-runtime-git-main.vercel.sh

Please sign in to comment.