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

types: fix Session interface reference #182

Merged
merged 1 commit into from Dec 28, 2022
Merged

Conversation

FabianFrank
Copy link
Contributor

@FabianFrank FabianFrank commented Dec 22, 2022

now Session actually resolves to the session interface defined in the same files instead of being an unresolved any. goal of this change is to fix #181

before:
Screenshot 2022-12-22 at 11 53 10 AM

after:
Screenshot 2022-12-22 at 11 53 22 AM

Checklist

This doesn't require a new test/benchmark or doc change to the best of my knowledge.

now Session actually resolves to the session interface defined in the same files instead of being an unresolved any
@Uzlopak
Copy link
Contributor

Uzlopak commented Dec 22, 2022

Actually we could need a typing test

See types.test-d.ts

@FabianFrank
Copy link
Contributor Author

@Uzlopak there's already a typing test for this at

expectType<{ id: number } | undefined>(session.user);
and the test doesn't break with this change. it's unfortunate that the types work under test but not when installed as a dependency in another project. frankly, I'm not familiar enough with tsd to figure out how to make the test fail with Fastify.Session.

@Uzlopak
Copy link
Contributor

Uzlopak commented Dec 22, 2022

I will have a look later

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR! Can you please add a unit test? We use tsd for typescript.

@FabianFrank
Copy link
Contributor Author

@mcollina please see #182 (comment)

@mcollina
Copy link
Member

I'm uncomfortable in landing this without a test.

@FabianFrank
Copy link
Contributor Author

FabianFrank commented Dec 24, 2022

@mcollina there is already a test for this type. how do you think the existing test for this type should change?

@mcollina
Copy link
Member

I don't know. I'm not the one proposing the change in this PR.

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

LGTM.

Let me explain a bit of the problem.
The local assertion will always pass since as a package point, we know Fastify types before using it.
However, when it comes to the consumer. It will create an infinite loop of referencing. So, it will return any as a result.

For a test, we need to create a dummy package.json, install itself as dependent. Then, assertion the types inside the dummy package. Would it be too heavy as a test?

@FabianFrank
Copy link
Contributor Author

In #181 (comment) we were able to track this down to the esModuleInterop setting in typescript, so to build on @climba03003's idea of a separate test package, we'd also have to make that test package an ESM module and disable module interop. Then the question is are we comfortable to only test the ESM case or do we want to test the CJS case as well, i.e. have 2 test packages?

@Eomm Eomm requested a review from mcollina December 28, 2022 16:25
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 94eaa84 into fastify:master Dec 28, 2022
@FabianFrank FabianFrank deleted the patch-1 branch December 28, 2022 18:53
bodinsamuel pushed a commit to specfy/specfy that referenced this pull request Aug 21, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@fastify/session](https://togithub.com/fastify/session) | [`10.2.0`
->
`10.4.0`](https://renovatebot.com/diffs/npm/@fastify%2fsession/10.2.0/10.4.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@fastify%2fsession/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@fastify%2fsession/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@fastify%2fsession/10.2.0/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@fastify%2fsession/10.2.0/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/session (@&#8203;fastify/session)</summary>

###
[`v10.4.0`](https://togithub.com/fastify/session/releases/tag/v10.4.0)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.3.1...v10.4.0)

#### What's Changed

- fix: make sure to set cookie on manual session saves by
[@&#8203;SimenB](https://togithub.com/SimenB) in
[fastify/session#203

**Full Changelog**:
fastify/session@v10.3.1...v10.4.0

###
[`v10.3.1`](https://togithub.com/fastify/session/releases/tag/v10.3.1)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.3.0...v10.3.1)

#### What's Changed

- ci: only trigger on pushes to main branches by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#198
- build(deps-dev): bump
[@&#8203;types/node](https://togithub.com/types/node) from 18.16.5 to
20.1.0 by [@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#199
- fix: manually persisting the session should make `isModified` return
false by [@&#8203;SimenB](https://togithub.com/SimenB) in
[fastify/session#201

**Full Changelog**:
fastify/session@v10.3.0...v10.3.1

###
[`v10.3.0`](https://togithub.com/fastify/session/releases/tag/v10.3.0)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.2.1...v10.3.0)

#### What's Changed

- build(deps-dev): bump sinon from 13.0.2 to 14.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#88
- update to Fastify v4 by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[fastify/session#89
- feat: Update benchmark. by
[@&#8203;ShogunPanda](https://togithub.com/ShogunPanda) in
[fastify/session#91
- ci: migrate to fastify reusable workflow by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#90
- build(deps-dev): bump fastify-cookie from 5.6.1 to 5.7.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#92
- docs(readme): remove snyk badge by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#93
- chore(.gitignore): use updated skeleton template by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#96
- build(deps-dev): bump
[@&#8203;types/node](https://togithub.com/types/node) from 17.0.45 to
18.0.0 by [@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#97
- build(deps-dev): bump tsd from 0.20.0 to 0.21.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#98
- build(deps-dev): bump tsd from 0.21.0 to 0.22.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#102
- style(types/types.test-d.ts): explicitly declare semicolon by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#103
- refactor: use optional chaining by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#104
- build(deps): bump fastify-plugin from 3.0.1 to 4.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#105
- remove metadata.js by [@&#8203;Uzlopak](https://togithub.com/Uzlopak)
in
[fastify/session#107
- separate lint from npm test script, activate lint and license-check in
ci pipeline by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#108
- refactor unit tests, replace ava with tap, remove undici and
typescript as devDependencies by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#109
- move benchmark to corresponding folder, slim it down by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#110
- tap should only run test.js files by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#115
- Clean up tests by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#117
- Add missing documentation for README.md by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#116
- Simplify shouldSaveSession code a little bit by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#119
- Move `expires` from session to session.cookies by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#120
- refactor store to es6 class by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#122
- simplify callback typing by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#124
- Add cookiePrefix as an option to allow for compatibility with
express-session by [@&#8203;rclmenezes](https://togithub.com/rclmenezes)
in
[fastify/session#113
- perf: improve idgenerator (about 25-30 % faster) by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#123
- chore: remove addDataToSession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#127
- add autocannon example by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#128
- feat: use
[@&#8203;fastify/cookie-signer-logic](https://togithub.com/fastify/cookie-signer-logic)
for signing and unsigning by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#129
- move signer require into ensureDefaults by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#130
- fix unit test by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#132
- fix lgtm alert by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#133
- Remove session id round 4 by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#134
- simplify getExpires, export getExpires from cookie by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#135
- remove internal used session restore() function by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#136
- Revert "refactor store to es6 class" by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#138
- remove sinon as dev dependency by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#139
- add redis example by [@&#8203;Uzlopak](https://togithub.com/Uzlopak)
in
[fastify/session#142
- add pre-commit by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#145
- add unit test for maxAge by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#144
- create a separate unit test file for memorystore by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#150
- rename secret.test.js to fastifySession.checkOptions.test.js and
refactor it by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#151
- prevent encryptedSessionId from being stored in the sessionStore by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#143
- use maxAge over expires by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#152
- build(deps-dev): bump tsd from 0.22.0 to 0.23.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#153
- Resave session cookie if saveUninitialized is false and rolling is
true by [@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#149
- rename session to fastifySession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#160
- Make it possible to call save() in decryptSession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#161
- Implement originalMaxAge by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#162
- Remove deprecation notice for fastify-Instance decryptSession by
[@&#8203;shrihari-prakash](https://togithub.com/shrihari-prakash) in
[fastify/session#164
- add typings for decryptSession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#165
- simplify cookie handling by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#166
- minor cosmetic change by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#167
- ensure that secure false is not overwritten by null by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#168
- security: verify path of cookie/session by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#169
- build(deps-dev): bump tsd from 0.23.0 to 0.24.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#171
- Little remark fastify-cookie ->
[@&#8203;fastify/cookie](https://togithub.com/fastify/cookie) by
[@&#8203;Luchanso](https://togithub.com/Luchanso) in
[fastify/session#172
- fix rolling location in doc by
[@&#8203;HPieters](https://togithub.com/HPieters) in
[fastify/session#175
- fix: set domain when clearing cookie by
[@&#8203;TheWashiba](https://togithub.com/TheWashiba) in
[fastify/session#174
- fix: align cookie implementation with express-session by
[@&#8203;climba03003](https://togithub.com/climba03003) in
[fastify/session#177
- nodenext compatibility by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#178
- build(deps-dev): bump tsd from 0.24.1 to 0.25.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#179
- types: fix Session interface reference by
[@&#8203;FabianFrank](https://togithub.com/FabianFrank) in
[fastify/session#182
- chore(.gitignore): add clinic by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#183
- chore(.gitignore): add bun lockfile by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#185
- build(deps-dev): bump connect-redis from 6.1.3 to 7.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#186
- build(deps-dev): bump tsd from 0.25.0 to 0.27.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#187
- build(deps-dev): bump tsd from 0.27.0 to 0.28.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#188
- fix: express session type compatibility by
[@&#8203;ojeytonwilliams](https://togithub.com/ojeytonwilliams) in
[fastify/session#190
- feat: add ignoreFields option to regenerate method by
[@&#8203;RafaelGSS](https://togithub.com/RafaelGSS) in
[fastify/session#189
- fix: separate an actual path from url search part before verification
by [@&#8203;ohchi](https://togithub.com/ohchi) in
[fastify/session#192
- fix: lint by [@&#8203;Eomm](https://togithub.com/Eomm) in
[fastify/session#195
- feat!: stricter types by
[@&#8203;climba03003](https://togithub.com/climba03003) in
[fastify/session#196

#### New Contributors

- [@&#8203;mcollina](https://togithub.com/mcollina) made their first
contribution in
[fastify/session#89
- [@&#8203;ShogunPanda](https://togithub.com/ShogunPanda) made their
first contribution in
[fastify/session#91
- [@&#8203;Uzlopak](https://togithub.com/Uzlopak) made their first
contribution in
[fastify/session#107
- [@&#8203;rclmenezes](https://togithub.com/rclmenezes) made their first
contribution in
[fastify/session#117
- [@&#8203;shrihari-prakash](https://togithub.com/shrihari-prakash) made
their first contribution in
[fastify/session#164
- [@&#8203;Luchanso](https://togithub.com/Luchanso) made their first
contribution in
[fastify/session#172
- [@&#8203;HPieters](https://togithub.com/HPieters) made their first
contribution in
[fastify/session#175
- [@&#8203;TheWashiba](https://togithub.com/TheWashiba) made their first
contribution in
[fastify/session#174
- [@&#8203;FabianFrank](https://togithub.com/FabianFrank) made their
first contribution in
[fastify/session#182
- [@&#8203;ojeytonwilliams](https://togithub.com/ojeytonwilliams) made
their first contribution in
[fastify/session#190
- [@&#8203;RafaelGSS](https://togithub.com/RafaelGSS) made their first
contribution in
[fastify/session#189
- [@&#8203;ohchi](https://togithub.com/ohchi) made their first
contribution in
[fastify/session#192
- [@&#8203;Eomm](https://togithub.com/Eomm) made their first
contribution in
[fastify/session#195

**Full Changelog**:
fastify/session@v8.3.0...v10.3.0

###
[`v10.2.1`](https://togithub.com/fastify/session/releases/tag/v10.2.1)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.2.0...v10.2.1)

#### What's Changed

- fix: separate an actual path from url search part before verification
by [@&#8203;ohchi](https://togithub.com/ohchi) in
[fastify/session#192
- fix: lint by [@&#8203;Eomm](https://togithub.com/Eomm) in
[fastify/session#195

#### New Contributors

- [@&#8203;ohchi](https://togithub.com/ohchi) made their first
contribution in
[fastify/session#192
- [@&#8203;Eomm](https://togithub.com/Eomm) made their first
contribution in
[fastify/session#195

**Full Changelog**:
fastify/session@v10.2.0...v10.2.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday" in timezone
Europe/Paris, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/specfy/specfy).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40My4yIiwidXBkYXRlZEluVmVyIjoiMzYuNDMuMiIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

request.session type is always any
5 participants