Skip to content

Conversation

@kateinoigakukun
Copy link
Member

No description provided.

@kateinoigakukun kateinoigakukun force-pushed the yt/improve-nodejs-experience branch from ed2be3a to fe6f380 Compare November 29, 2025 06:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a Node.js example demonstrating JavaScriptKit usage in Node.js environments and introduces comprehensive documentation about the package output structure generated by the swift package js command. The changes make the platform setup functions more flexible by making their parameters optional with sensible defaults.

Key changes:

  • New documentation article explaining the structure and usage of generated JavaScript packages
  • Node.js example showing how to instantiate and run Swift code in Node.js
  • Optional parameters for defaultNodeSetup() and defaultBrowserSetup() functions to simplify common use cases

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Sources/JavaScriptKit/Documentation.docc/Documentation.md Adds link to new Package Output Structure article
Sources/JavaScriptKit/Documentation.docc/Articles/Package-Output-Structure.md New comprehensive documentation covering package structure, core files, and usage examples for browser and Node.js
Plugins/PackageToJS/Templates/platforms/node.js Makes options parameter optional with default value and provides fallback for spawnWorker
Plugins/PackageToJS/Templates/platforms/node.d.ts Updates type definitions to reflect optional parameters
Plugins/PackageToJS/Templates/platforms/browser.js Adds optional chaining for options parameter access (has implementation issues)
Plugins/PackageToJS/Templates/platforms/browser.d.ts Makes options and spawnWorker parameters optional
Examples/NodeJS/main.mjs Node.js example demonstrating Swift/JS interop
Examples/NodeJS/Sources/NodeJS/NodeJS.swift Swift code that exports a greeting function to JavaScript
Examples/NodeJS/README.md Instructions for running the Node.js example
Examples/NodeJS/Package.swift Swift package manifest for Node.js example
Examples/NodeJS/.gitignore Standard Swift project gitignore file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

module: options?.module,
/* #if HAS_IMPORTS */
getImports() { return options.getImports() },
getImports() { return options?.getImports() },
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

When HAS_IMPORTS is defined, the getImports function is called unconditionally, but options may be undefined. This will cause a runtime error: Cannot read properties of undefined (reading 'getImports').

Consider using optional chaining with a fallback: return options?.getImports?.() ?? {}

Suggested change
getImports() { return options?.getImports() },
getImports() { return options?.getImports?.() ?? {} },

Copilot uses AI. Check for mistakes.

return {
module: options.module,
module: options?.module,
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The module property is required by InstantiateOptions but can be undefined when options is not provided. This could lead to runtime errors during instantiation.

Consider either requiring module in the options parameter or providing a sensible default/error handling.

Copilot uses AI. Check for mistakes.
kateinoigakukun and others added 2 commits November 29, 2025 15:52
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@kateinoigakukun kateinoigakukun merged commit 6892354 into main Nov 29, 2025
8 checks passed
@kateinoigakukun kateinoigakukun deleted the yt/improve-nodejs-experience branch November 29, 2025 06:53
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

Successfully merging this pull request may close these issues.

2 participants