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

Add missing typings #33181

Closed
wants to merge 2 commits into from
Closed

Add missing typings #33181

wants to merge 2 commits into from

Conversation

tmkx
Copy link

@tmkx tmkx commented Jan 11, 2022

Next.js has provided a way to do internationalization, but it's URL-based, different locales have a different route.

It doesn't meet all scenes, so I detect the user's Accept-Languages header to choose a locale, or users can persist theirs prefer locale in Cookies. I did this in App.getInitialProps, read cookies, and then render the right locale, but the AppContext's req doesn't have cookies typings, so I think it's maybe "missing"?

cookies were processed in:

setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers))

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running yarn lint

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Hi, this change looks good, can we add a test for this potentially to this test suite ensuring the type is valid on a page with getServerSideProps?

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

Can you add a clear description to the PR body, it's currently not super clear what this is fixing.

@tmkx
Copy link
Author

tmkx commented Jan 13, 2022

Can you add a clear description to the PR body, it's currently not super clear what this is fixing.

@timneutkens my bad, updated.

@tmkx
Copy link
Author

tmkx commented Jan 21, 2022

fixed by #32999

@tmkx tmkx closed this Jan 21, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants