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

refactor: use @copilot-extensions/preview-sdk #3

Merged
merged 7 commits into from
Aug 27, 2024
Merged

refactor: use @copilot-extensions/preview-sdk #3

merged 7 commits into from
Aug 27, 2024

Conversation

gr2m
Copy link
Collaborator

@gr2m gr2m commented Aug 26, 2024

This pull request

  • adds a new dependency: @copilot-extensions/preview-sdk
  • refactors existing code to be an ES Module. Unfortunately the .js extension is necessary for imports in ESM apps.
  • removes the custom ceviry function and uses the SDK instead

The SDK will have to be a native ES Module. Octokit was updated to ESM across all its SDKs. So we should encourage integrators to build apps as ES Modules from the getgo if they want to take advantage of the new SDK and the latest @octokit

The only relevant change is 89233db, it shows the code that can be removed form the app.

@gr2m
Copy link
Collaborator Author

gr2m commented Aug 26, 2024

Hmm the dev server fails now, any idea?

$ npm run dev

> [email protected] dev
> nodemon src/index.ts

[nodemon] 3.1.4
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: ts,json
[nodemon] starting `ts-node src/index.ts`
TypeError: Unknown file extension ".ts" for /Users/gr2m/code/copilot-extensions/github-models-extension/src/index.ts
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
    at defaultLoad (node:internal/modules/esm/load:143:22)
    at async ModuleLoader.load (node:internal/modules/esm/loader:553:7)
    at async ModuleLoader.moduleProvider (node:internal/modules/esm/loader:434:45)
    at async link (node:internal/modules/esm/module_job:87:21) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}
[nodemon] app crashed - waiting for file changes before starting...

@JasonEtco
Copy link
Collaborator

Unfortunately the .js extension is necessary for imports in ESM apps.

I would expect the TS compiler to handle this for us? That's likely why the dev script is failing.

Copy link
Collaborator

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

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

Fixed it by moving to use tsx instead of nodemon/ts-node, which has better support for ESM out of the box. Starts up fine now.

@JasonEtco JasonEtco merged commit 0c81f57 into main Aug 27, 2024
2 checks passed
@JasonEtco JasonEtco deleted the sdk branch August 27, 2024 01:59
@gr2m
Copy link
Collaborator Author

gr2m commented Aug 27, 2024

I would expect the TS compiler to handle this for us?

It would need more than transpiling, because the underlying node code cannot import files without file extensions. The build step would take each relative import path and find out to which file it resolves, then dynamically add the file extension or /index.js in case of folders. It might be out of scope of TypeScript itself, but it sure feels odd to have .js extensions for .ts source files.

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