Skip to content
This repository was archived by the owner on Sep 29, 2025. It is now read-only.

Conversation

@admah
Copy link

@admah admah commented Jan 12, 2024

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Jan 12, 2024

⚠️ No Changeset found

Latest commit: 9da5c23

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

🧪 Prereleases are available for testing 🧪

Details

@cloudflare/next-on-pages

You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/7507981567/npm-package-next-on-pages-633

@cloudflare/eslint-plugin-next-on-pages

You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/7507981567/npm-package-eslint-plugin-next-on-pages-633

Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

lgtm. the line numbers will likely change over time, but this type needs to be documented in wrangler docs as part of getBindingsProxy anyway we can keep it as is and update the link here then.

@IgorMinar
Copy link
Contributor

@admah can you please fix the prettier check failure?

```


> View [this file](https://github.com/cloudflare/next-on-pages/blob/main/internal-packages/next-dev/src/index.ts#L31-L60) for a list of available binding types.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> View [this file](https://github.com/cloudflare/next-on-pages/blob/main/internal-packages/next-dev/src/index.ts#L31-L60) for a list of available binding types.
> View [these types](https://github.com/cloudflare/next-on-pages/blob/main/internal-packages/next-dev/src/index.ts#L31-L60) for a list of available binding types.

or something along those lines as user should not care about the whole file no? 🤔

@dario-piotrowicz
Copy link
Member

the line numbers will likely change over time

For that reason I always go with permalinks, so that the link itself will always remain valid, wouldn't then something like
https://github.com/cloudflare/next-on-pages/blob/8e93067/internal-packages/next-dev/src/index.ts#L31-L60
be better?

@dario-piotrowicz
Copy link
Member

I am actually not sure of this change... I think that presenting the bindings in the md file instead of pointing users to a ts file would be a very simple, quick and nicer alternative to this 🤔 (we could also add docs links to the binding types, etc...)

Note

You can notice that the binding types lack ts-docs, I didn't add them since as Igor mentioned, the logic here should be replaced with getBindingsProxy soon

@admah if you wouldn't mind I could give that a go?

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Feb 1, 2024

I'm closing this PR since we're moving away from the setupDevBindings and the types references by the PR (see: #648)

I'll make sure to update the next-on-pages docs with all the available bindings 🙂

Or actually I'll just refer to the getBindingsProxy documentation: e604c08

I hope that works for you @admah 🙂🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants