Skip to content

Conversation

atinux
Copy link
Contributor

@atinux atinux commented Jan 11, 2021

TypeScript

The module has been rewritten with TypeScript and using Siroc as bundler.

Session options

This PR also add a properties into the module to configure the session:

export default {
  strapi: {
    key: 'strapi_jwt',
    expires: 'session',
    cookie: {}
  }
}

Resolves #92 and #85

key

  • Type: String
  • Default: 'strapi_jwt'

Key used for the cookie name as well as LocalStorage/SessionStorage key.

expires

  • Type: String or Number or 'session'
  • Default: 'session'

When expires === 'session', the sessionStorage will be used to act like the cookie.

Otherwise, the value is a string, it will be parsed using ms, ex: expires: '7d'

A number can also be provided as milliseconds, ex: expires: 7 * 24 * 3600 * 1000

It would be possible also to configure during runtime the expiration, example:

if (form.rememberMe) {
  this.$strapi.options.expires = 31 * 24 * 3600 * 1000; // Only number is possible
}
await this.$strapi.login({ identifier: '...', password: '...' })

cookie

  • Type: Object
  • Default: {}

Options to forward to the cookie#options module, the expires property will be overwritten.

Resolved #70

Misc

Help needed

  • Vue is bundled into dist/runtime.mjs, I had to list it into peerDependencies but I am not found of this solution (NPM like to warn if no direct dependencies of the user project), any hint @danielroe?
  • The typings are not correctly generated into dist/types.d.ts, they are missing the one declared into src/types
  • I need help regarding types/strapi.ts, I think it is not optimal, would love some feedback from @pi0 and @LuckeeDev
  • The runtime does not seem to work when I yarn link @nuxtjs/strapi
    Screenshot 2021-01-11 at 18 48 50

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes (if not applicable, please state why)

@atinux atinux requested review from danielroe and pi0 January 11, 2021 17:51
@atinux atinux linked an issue Jan 11, 2021 that may be closed by this pull request
@LuckeeDev
Copy link
Contributor

LuckeeDev commented Jan 11, 2021

Great stuff here! I'm more than happy to help on this.

Just started going through the changes, specifically the ones regarding the type system. May I suggest other changes as I come across other issues?

I could also help on the new docs, if needed.

Copy link
Contributor Author

atinux commented Jan 11, 2021

Just started going through the changes, specifically the ones regarding the type system. May I suggest other changes as I come across other issues?

Sure thing 😄

I could also help on the new docs, if needed.

That would be awesome 🔥

@atinux
Copy link
Contributor Author

atinux commented Jan 11, 2021

I updated the PR description based on my last commit: flattening the props (removing session level) and adding the possibility to update the expiration at runtime.

@pi0
Copy link
Contributor

pi0 commented Jan 11, 2021

One suggestion, can we pass option to login instead of directly modifying options?

Example:

await this.$strapi.login({
     identifier: form.identifier,
     password: form.password,
     expires: form.rememberMe && 31 * 24 * 3600 * 1000 
 })

Or alrternatively with a default value of expires:

await this.$strapi.login({
     identifier: form.identifier,
     password: form.password,
     rememberMe : !!form.rememberMe
 })

@atinux
Copy link
Contributor Author

atinux commented Jan 12, 2021

After talking with Ben, we will go with $strapi.login(credentials, options), will make a commit this morning.

@LuckeeDev
Copy link
Contributor

I refactored a bunch of the type declarations and the way the .d.ts files work, how can I add a commit to this PR? Or should I just add reviews to the code you wrote?

@LuckeeDev
Copy link
Contributor

Tomorrow I might work on some of the new docs.

@atinux
Copy link
Contributor Author

atinux commented Jan 12, 2021

@LuckeeDev you should be able to push on this branch now :)

@LuckeeDev
Copy link
Contributor

Awesome! I'll commit later this evening.

@LuckeeDev
Copy link
Contributor

I've rewritten the type definitions to be extracted directly from the .ts files inside .d.ts declaration files. The module doesn't work with the current version of the example though.

Copy link
Contributor Author

atinux commented Jan 13, 2021

Thank you @LuckeeDev

What do you mean by the module doesn't work with the current version of the example?

Copy link
Contributor Author

atinux commented Jan 13, 2021

I added the options in the documentation, do we need to change anything regarding https://github.com/nuxt-community/strapi-module/blob/master/docs/content/en/typescript.md @LuckeeDev ?

@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #96 (658c0c7) into master (8e4a347) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #96   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           14        17    +3     
  Branches         3         4    +1     
=========================================
+ Hits            14        17    +3     
Impacted Files Coverage Δ
src/module.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e4a347...658c0c7. Read the comment docs.

@LuckeeDev
Copy link
Contributor

LuckeeDev commented Jan 14, 2021

Thank you @LuckeeDev

What do you mean by the module doesn't work with the current version of the example?

I'll be checking tomorrow!

Copy link
Contributor Author

atinux commented Jan 15, 2021

Thank you @LuckeeDev, waiting for you last feedback before merging

@LuckeeDev
Copy link
Contributor

I think We're good to go... anything else to do?

atinux and others added 2 commits January 19, 2021 10:49
Co-authored-by: Luca Zoppetti <[email protected]>
Co-authored-by: Luca Zoppetti <[email protected]>
@atinux atinux merged commit de5be3b into master Jan 19, 2021
@atinux atinux deleted the feat/improvements branch January 19, 2021 10:49
@atinux
Copy link
Contributor Author

atinux commented Jan 19, 2021

Thank you so much guys for your precious help 💚

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

Labels

None yet

Projects

None yet

3 participants