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.

Sign up for GitHub

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

Export Cloudflare runtime types #5230

Merged
merged 2 commits into from
Oct 28, 2022
Merged

Export Cloudflare runtime types #5230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
943f10f
Export Cloudflare runtime types
matthewp Oct 28, 2022
be1a1e4
Adding a changeset
matthewp Oct 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/new-pumas-begin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/cloudflare': patch
---

Exports new runtime entrypoint's types
5 changes: 4 additions & 1 deletion packages/integrations/cloudflare/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
"homepage": "https://docs.astro.build/en/guides/integrations-guide/cloudflare/",
"exports": {
".": "./dist/index.js",
"./runtime": "./dist/runtime.js",
"./runtime": {
"types": "./runtime.d.ts",
"default": "./dist/runtime.js"
},
"./server.advanced.js": "./dist/server.advanced.js",
"./server.directory.js": "./dist/server.directory.js",
"./package.json": "./package.json"
Expand Down
8 changes: 8 additions & 0 deletions packages/integrations/cloudflare/runtime.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export type {
Copy link
Member

Choose a reason for hiding this comment

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

Looks good! What's the reason for this manual .d.ts instead of emitting types when building to dist/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do emit types to dist. But TypeScript doesn't read the exports field (in certain versions of TypeScript) and so it has to find them at the root like this.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh, right — would it make sense to update the build script to astro-scripts build \"src/**/*.ts\" && tsc && cp dist/runtime.d.ts runtime.d.ts and .gitignore the one in root?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this approach is simpler than depending on a build that could be changed accidentally.

Copy link
Member

Choose a reason for hiding this comment

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

I thought the manual version might be prone to becoming out of date if we change src and not this file, but if the runtime file won’t change very often maybe that’s fine 👍

Copy link
Contributor Author

@matthewp matthewp Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that is a risk. We are doing this the same way in the main astro package. Not sure if there's a good solution.

WorkerRuntime,
PagesRuntime
} from './dist/runtime';

export {
getRuntime
} from './dist/runtime';
bluwy marked this conversation as resolved.
Show resolved Hide resolved