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

Re-evaluating Node.js Experimental Features #285

Open
RafaelGSS opened this issue Jun 14, 2024 · 17 comments
Open

Re-evaluating Node.js Experimental Features #285

RafaelGSS opened this issue Jun 14, 2024 · 17 comments

Comments

@RafaelGSS
Copy link
Member

RafaelGSS commented Jun 14, 2024

I believe it's time to discuss our policies around experimental features in the project. Features like --experimental-network-import are challenging to assess and maintain, especially when there is no active champion or creator overseeing them.

Proposal

I propose that when the creator or champion of an experimental feature becomes inactive, we should:

  1. Re-evaluate the benefits of the feature.
  2. Decide whether to remove the feature entirely or delegate its maintenance to a team.

This approach will help us set clear expectations, assess and review vulnerabilities, and ensure better maintenance of our experimental features.

Recent Example

We recently dropped the --experimental-policy feature because:

  • We were receiving reports about it.
  • The documentation did not clearly define its boundaries and security expectations, making it difficult to assess.

Actionable Steps

To facilitate this discussion, I have written a simple script to list all experimental APIs from nodejs/node/docs/api/**. You can find it here.

Can we evaluate these experimental features either asynchronously or through a dedicated call?

File API Group Header Re-Evaluated
doc/api/async_context.md AsyncLocalStorage.bind(fn) AsyncLocalStorage
doc/api/async_context.md AsyncLocalStorage.snapshot() AsyncLocalStorage
doc/api/async_context.md asyncLocalStorage.disable() AsyncLocalStorage
doc/api/async_context.md asyncLocalStorage.enterWith(store) AsyncLocalStorage
doc/api/async_context.md asyncLocalStorage.exit(callback[, ...args]) AsyncLocalStorage
doc/api/async_hooks.md AsyncHooks Async hooks
doc/api/buffer.md buffer.resolveObjectURL(id) Buffer
doc/api/child_process.md subprocess[Symbol.dispose]() Child Process
doc/api/cli.md --allow-addons Permission Model
doc/api/cli.md --allow-child-process Permission Model
doc/api/cli.md --allow-fs-read Permission Model
doc/api/cli.md --allow-fs-write Permission Model
doc/api/cli.md --allow-wasi Permission Model
doc/api/cli.md --allow-worker Permission Model
doc/api/cli.md --build-snapshot Startup Snapshot
doc/api/cli.md --build-snapshot-config Startup Snapshot
doc/api/cli.md -C condition, --conditions=condition Conditional Exports
doc/api/cli.md --cpu-prof
doc/api/cli.md --cpu-prof-dir
doc/api/cli.md --cpu-prof-interval
doc/api/cli.md --cpu-prof-name
doc/api/cli.md --disable-warning=code-or-type
doc/api/cli.md --expose-gc
doc/api/cli.md --env-file=config Node Env
doc/api/cli.md --experimental-default-type=type Loaders
doc/api/cli.md --experimental-detect-module Loaders
doc/api/cli.md --experimental-network-imports Loaders
doc/api/cli.md --experimental-permission Permission Model
doc/api/cli.md --experimental-require-module Loaders
doc/api/cli.md --experimental-sea-config SEA
doc/api/cli.md --experimental-shadow-realm
doc/api/cli.md --experimental-test-module-mocks Test Runner
doc/api/cli.md --experimental-test-snapshots Test Runner
doc/api/cli.md --frozen-intrinsics
doc/api/cli.md --heap-prof
doc/api/cli.md --heap-prof-dir
doc/api/cli.md --heap-prof-interval
doc/api/cli.md --heap-prof-name
doc/api/cli.md --heapsnapshot-near-heap-limit=max_count
doc/api/cli.md --import=module Loaders
doc/api/cli.md --jitless
doc/api/cli.md --no-experimental-global-navigator
doc/api/cli.md --run
doc/api/cli.md --snapshot-blob=path Startup Snapshot
doc/api/cli.md --test-update-snapshots
doc/api/cli.md NODE_COMPILE_CACHE=dir
doc/api/cli.md Source map cache
doc/api/corepack.md Corepack Corepack
doc/api/crypto.md crypto.hash(algorithm, data[, outputEncoding]) Crypto
doc/api/dgram.md socket[Symbol.asyncDispose]()
doc/api/diagnostics_channel.md diagnostics_channel.tracingChannel(nameOrChannels) Diagnostics
doc/api/diagnostics_channel.md channel.bindStore(store[, transform]) Diagnostics
doc/api/diagnostics_channel.md channel.unbindStore(store) Diagnostics
doc/api/diagnostics_channel.md channel.runStores(context, fn[, thisArg[, ...args]]) Diagnostics
doc/api/diagnostics_channel.md Class: TracingChannel Diagnostics
doc/api/diagnostics_channel.md tracingChannel.subscribe(subscribers) Diagnostics
doc/api/diagnostics_channel.md tracingChannel.unsubscribe(subscribers) Diagnostics
doc/api/diagnostics_channel.md tracingChannel.traceSync(fn[, context[, thisArg[, ...args]]]) Diagnostics
doc/api/diagnostics_channel.md tracingChannel.tracePromise(fn[, context[, thisArg[, ...args]]]) Diagnostics
doc/api/diagnostics_channel.md tracingChannel.traceCallback(fn, position, context, thisArg, ...args) Diagnostics
doc/api/diagnostics_channel.md Built-in Channels
doc/api/errors.md ERR_INPUT_TYPE_NOT_ALLOWED
doc/api/errors.md ERR_REQUIRE_CYCLE_MODULE
doc/api/errors.md ERR_REQUIRE_ASYNC_MODULE
doc/api/errors.md ERR_REQUIRE_ESM
doc/api/errors.md ERR_UNKNOWN_FILE_EXTENSION
doc/api/errors.md ERR_UNKNOWN_MODULE_FORMAT
doc/api/errors.md ERR_USE_AFTER_CLOSE
doc/api/errors.md ERR_NETWORK_IMPORT_BAD_RESPONSE
doc/api/errors.md ERR_NETWORK_IMPORT_DISALLOWED
doc/api/esm.md import.meta.dirname
doc/api/esm.md import.meta.filename
doc/api/esm.md import.meta.resolve(specifier)
doc/api/esm.md JSON modules ESM
doc/api/esm.md Wasm modules ESM
doc/api/esm.md HTTPS and HTTP imports ESM
doc/api/events.md events.addAbortListener(signal, listener) Events
doc/api/fs.md filehandle.readableWebStream([options]) File System
doc/api/fs.md filehandle[Symbol.asyncDispose]() File System
doc/api/fs.md fsPromises.glob(pattern[, options]) File System
doc/api/fs.md fs.glob(pattern[, options], callback) File System
doc/api/fs.md fs.openAsBlob(path[, options]) File System
doc/api/fs.md fs.globSync(pattern[, options]) File System
doc/api/fs.md dirent.parentPath File System
doc/api/globals.md Class: ByteLengthQueuingStrategy
doc/api/globals.md Class: CompressionStream
doc/api/globals.md Class: CountQueuingStrategy
doc/api/globals.md Crypto Crypto
doc/api/globals.md CryptoKey Crypto
doc/api/globals.md CustomEvent
doc/api/globals.md Class: DecompressionStream
doc/api/globals.md Navigator
doc/api/globals.md Class: ReadableByteStreamController
doc/api/globals.md Class: ReadableStream
doc/api/globals.md Class: ReadableStreamBYOBReader
doc/api/globals.md Class: ReadableStreamBYOBRequest
doc/api/globals.md Class: ReadableStreamDefaultController
doc/api/globals.md Class: ReadableStreamDefaultReader
doc/api/globals.md SubtleCrypto
doc/api/globals.md Class: TextDecoderStream
doc/api/globals.md Class: TextEncoderStream
doc/api/globals.md TransformStream TransformStream
doc/api/globals.md TransformStreamDefaultController TransformStreamDefaultController
doc/api/globals.md WebSocket globals.md
doc/api/globals.md WritableStream WritableStream
doc/api/globals.md WritableStreamDefaultController WritableStreamDefaultController
doc/api/globals.md WritableStreamDefaultWriter WritableStreamDefaultWriter
doc/api/http.md serverSymbol.asyncDispose http.md
doc/api/http2.md serverSymbol.asyncDispose http2.md
doc/api/https.md serverSymbol.asyncDispose https.md
doc/api/inspector.md Promises API inspector.md
doc/api/module.md module.register(specifier[, parentURL][, options]) module.md
doc/api/module.md Customization Hooks module.md
doc/api/module.md initialize() module.md
doc/api/module.md resolve(specifier, context, nextResolve) module.md
doc/api/module.md load(url, context, nextLoad) module.md
doc/api/module.md Source map v3 support module.md
doc/api/n-api.md node_api_nogc_env n-api.md
doc/api/n-api.md node_api_nogc_finalize n-api.md
doc/api/n-api.md node_api_create_external_string_latin1 n-api.md
doc/api/n-api.md node_api_create_external_string_utf16 n-api.md
doc/api/n-api.md node_api_create_property_key_utf16 n-api.md
doc/api/n-api.md node_api_post_finalizer n-api.md
doc/api/net.md serverSymbol.asyncDispose net.md
doc/api/packages.md Determining package manager packages.md
doc/api/packages.md "packageManager" packages.md
doc/api/process.md process.constrainedMemory() process.md
doc/api/process.md process.availableMemory() process.md
doc/api/process.md process.getActiveResourcesInfo() process.md
doc/api/process.md process.loadEnvFile(path) process.md
doc/api/process.md process.setSourceMapsEnabled(val) process.md
doc/api/process.md process.sourceMapsEnabled process.md
doc/api/readline.md Promises API readline.md
doc/api/single-executable-applications.md Single executable applications single-executable-applications.md
doc/api/stream.md writable.writableAborted stream.md
doc/api/stream.md readable.readableAborted stream.md
doc/api/stream.md readable.readableDidRead stream.md
doc/api/stream.md readableSymbol.asyncDispose stream.md
doc/api/stream.md readable.compose(stream[, options]) stream.md
doc/api/stream.md readable.iterator([options]) stream.md
doc/api/stream.md readable.map(fn[, options]) stream.md
doc/api/stream.md readable.filter(fn[, options]) stream.md
doc/api/stream.md readable.forEach(fn[, options]) stream.md
doc/api/stream.md readable.toArray([options]) stream.md
doc/api/stream.md readable.some(fn[, options]) stream.md
doc/api/stream.md readable.find(fn[, options]) stream.md
doc/api/stream.md readable.every(fn[, options]) stream.md
doc/api/stream.md readable.flatMap(fn[, options]) stream.md
doc/api/stream.md readable.drop(limit[, options]) stream.md
doc/api/stream.md readable.take(limit[, options]) stream.md
doc/api/stream.md readable.reduce(fn[, initial[, options]]) stream.md
doc/api/stream.md stream.compose(...streams) stream.md
doc/api/stream.md stream.Readable.fromWeb(readableStream[, options]) stream.md
doc/api/stream.md stream.Readable.isDisturbed(stream) stream.md
doc/api/stream.md stream.isErrored(stream) stream.md
doc/api/stream.md stream.isReadable(stream) stream.md
doc/api/stream.md stream.Readable.toWeb(streamReadable[, options]) stream.md
doc/api/stream.md stream.Writable.fromWeb(writableStream[, options]) stream.md
doc/api/stream.md stream.Writable.toWeb(streamWritable) stream.md
doc/api/stream.md stream.Duplex.fromWeb(pair[, options]) stream.md
doc/api/stream.md stream.Duplex.toWeb(streamDuplex) stream.md
doc/api/test.md Watch mode test.md
doc/api/test.md Collecting code coverage test.md
doc/api/test.md Snapshot testing test.md
doc/api/test.md snapshot test.md
doc/api/test.md snapshot.setDefaultSnapshotSerializers(serializers) test.md
doc/api/test.md snapshot.setResolveSnapshotPath(fn) test.md
doc/api/test.md MockModuleContext test.md
doc/api/test.md mock.module(specifier[, options]) test.md
doc/api/test.md MockTimers test.md
doc/api/test.md context.assert.snapshot(value[, options]) test.md
doc/api/test.md context.plan(count) test.md
doc/api/timers.md immediateSymbol.dispose timers.md
doc/api/timers.md timeoutSymbol.dispose timers.md
doc/api/timers.md timersPromises.scheduler.wait(delay[, options]) timers.md
doc/api/timers.md timersPromises.scheduler.yield() timers.md
doc/api/tracing.md Trace events tracing.md
doc/api/url.md URL.createObjectURL(blob) url.md
doc/api/url.md URL.revokeObjectURL(id) url.md
doc/api/util.md util.MIMEType util.MIMEType
doc/api/util.md util.parseEnv(content) util.md
doc/api/util.md util.styleText(format, text) util.md
doc/api/util.md util.transferableAbortController() util.md
doc/api/util.md util.transferableAbortSignal(signal) util.md
doc/api/util.md util.aborted(signal, resource) util.md
doc/api/v8.md v8.queryObjects(ctor[, options]) v8.md
doc/api/v8.md v8.setHeapSnapshotNearHeapLimit(limit) v8.md
doc/api/v8.md Startup Snapshot API v8.md
doc/api/vm.md vm.Module vm.Module
doc/api/vm.md vm.SourceTextModule vm.SourceTextModule
doc/api/vm.md vm.SyntheticModule vm.SyntheticModule
doc/api/vm.md vm.constants.USE_MAIN_CONTEXT_DEFAULT_LOADER vm.md
doc/api/vm.md vm.measureMemory([options]) vm.md
doc/api/wasi.md WebAssembly System Interface (WASI) wasi.md
doc/api/webcrypto.md Ed25519/Ed448/X25519/X448 key pairs webcrypto.md
doc/api/worker_threads.md port.hasRef() worker_threads.md

(Feel free to edit this table)

@gireeshpunathil
Copy link
Member

Re-evaluate the benefits of the feature.
Decide whether to remove the feature entirely or delegate its maintenance to a team.

  • may be, we could also assess where does the feature stand in terms of its progress towards stability, what are the gaps and what are the opportunities?
  • may be, we could also assess where does the feature stand in terms of field adoption, what are the use cases and what the users think of about it?

@RafaelGSS
Copy link
Member Author

Absolutely @gireeshpunathil.

@BridgeAR
Copy link
Member

About having a call: I don't think a general call is useful in this case. The topics are driven by different groups of people e.g., crypto, test runner and AsyncLocalStorage. I think it would be good to try to get feedback by each group about the individual status of these.

@GeoffreyBooth
Copy link
Member

I think it would be good to try to get feedback by each group about the individual status of these.

I think a separate issue with experimental features by topic/team would be good. Maybe a goal would be that for every feature we're leaving as experimental, we both define its stage and create a tracking issue with a to-do list of what's left before it can go stable. We've had tracking issues for several recent experimental features and I've found them useful, and they're good for communicating to users what to expect.

@ronag
Copy link
Member

ronag commented Jun 17, 2024

  • AsyncHooks: Is there any point in still having this as experimental? IMHO it's not like it's going to be removed. In practice we are treating it as stable. It should either be doc deprecated or stable. Keeping it experimental is confusing.
  • All symbol.dispose should IMO stay experimental until it's more mainstream in javascript in general.
  • All web streams should IMO stay experimental until we have an active maintainer.

As I've mentioned many times before we have a more general problem with experimental features in terms of that we officially say that experimental features may change or be removed while in practice we don't want to do that due to breaking the ecosystem.

@mcollina
Copy link
Member

I propose:

  1. mark async_hooks stable.
  2. keep dispose experimental until it's settled in the spec (this is true for all other spec'd things, I think we can make it a rule)
  3. mark Web Streams as stable. There are enough people chipping in that I think it's time. Also it's very weird they are not becuase they are the basis for fetch().

@ronag
Copy link
Member

ronag commented Jun 17, 2024

  1. mark async_hooks stable.

I agree with this. However, are we good with that they don't work properly with esm? Or do they?

@mcollina
Copy link
Member

However, are we good with that they don't work properly with esm? Or do they?

I have a good feeling that they work better now that most of the loader is sync. We should do another test for that.

@RafaelGSS
Copy link
Member Author

What if we start top-to-bottom?

  • @Qard, what are your thoughts on AsyncLocalStorage (cc: @nodejs/diagnostics

For AsyncHooks we have the following mention on the docs:

Stability: 1 - Experimental. Please migrate away from this API, if you can. We do not recommend using the createHook, AsyncHook, and executionAsyncResource APIs as they have usability issues, safety risks, and performance implications. Async context tracking use cases are better served by the stable AsyncLocalStorage API. If you have a use case for createHook, AsyncHook, or executionAsyncResource beyond the context tracking need solved by AsyncLocalStorage or diagnostics data currently provided by Diagnostics Channel, please open an issue at https://github.com/nodejs/node/issues describing your use case so we can create a more purpose-focused API.

Having API with such a message, is that ok to move to stable? It seems Experimental will always be the place for it until we migrate things away from async_hooks.

@RafaelGSS
Copy link
Member Author

Also, I'll go ahead and mark all flags related to the Permission Model as "Re-Evaluated ✅" and keep its status as experimental as we are still developing and getting more feedback along the way.

@GeoffreyBooth
Copy link
Member

This thread is going to become unmanageable if we discuss specific APIs here. Can we get a new issue for async hooks?

@Qard
Copy link
Member

Qard commented Jun 17, 2024

I would doc-deprecate async_hooks and focus on the migration away from it for AsyncLocalStorage. While it is largely API stable, it's most certainly not stable from the perspective of being a thing you can trust to not crash your app. It's simply not a safe API and its use should not be encouraged.

I had asked previously for use cases not solved by ALS so we could build out some more purpose-focused APIs. There's also probably a use for a resource-tracking API which is reduced to only doing that without conflating it with promises and non-consumable resource types like http parser.

I would strongly suggest not stabilizing async_hooks in the state it is in presently. 😬

@anonrig
Copy link
Member

anonrig commented Jun 17, 2024

--run can stay the same. just quite recently, I bumped it to release candidate. in a couple of releases, I plan to make it stable.

@joyeecheung
Copy link
Member

joyeecheung commented Jun 18, 2024

Maybe we should create a new repo for this? And we PR the evaluation as documents that can be easily looked up by posterity?

(or reuse next-10 repo?)

@GeoffreyBooth
Copy link
Member

Maybe we should create a new repo for this? And we PR the evaluation as documents that can be easily looked up by posterity?

This is a great idea. The evaluation could also include the todo list for what work remains before a feature can become stable.

@RafaelGSS
Copy link
Member Author

@nodejs/next-10 can we transfer it to Next-10 and start creating issues to debate each topic? It should be async, no need to add to the agenda.

@marco-ippolito
Copy link
Member

@nodejs/next-10 can we transfer it to Next-10 and start creating issues to debate each topic? It should be async, no need to add to the agenda.

SGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants