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

refactor: migrate to adonisjs 6 #24

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

refactor: migrate to adonisjs 6 #24

wants to merge 12 commits into from

Conversation

Julien-R44
Copy link
Member

Changes

Add support for AdonisJS 6

No breaking changes, except the way we should import things :

import slugify from '@adonisjs/lucid-slugify/decorator'

class Post extends BaseModel {
  @column()
  @slugify({
    strategy: 'dbIncrement',
    fields: ['title']
  })
  public slug: string

  @column()
  public title: string
}

Types are now available on /types submodule :

import { SlugifyConfig, SlugifyStrategy } from '@adonisjs/lucid-slugify/types'

And SlugifyManager should be resolved from the container like that :

import type { ApplicationService } from '@adonisjs/core/types'

export default class AppProvider {
  constructor(protected app: ApplicationService) {}

  public async boot() {
    const slugify = await this.app.container.make('slugify.manager')

    slugify.extend('strategyName', (slugify, config) => {
      return new MyStrategy(config)
    })
  }
}

See README for more details.

@Julien-R44 Julien-R44 marked this pull request as draft March 15, 2024 15:11

await app.booted(async () => {
slugify = await app.container.make('slugify.decorator')
})
Copy link
Member Author

@Julien-R44 Julien-R44 Mar 15, 2024

Choose a reason for hiding this comment

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

not sure about this.

in fact, here we're making a container service for the decorator. This container service can be imported and used like this:

import slugify from '@adonisjs/lucid-slugify/decorator'

class Post {
  @slugify({ .. })
  public slug: string
}

it works, but I think it feel a bit weird because so far we've never used a container service for this kind of thing. but maybe its fine

it's the only solution I have found because the decorator needs SlugifyManager, and the IoC manages its instantiation.

if a better solution is desirable, please let me know

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that seems awful. The decorators shouldn't be turned into container services. Seems like, we will need a bit of refactoring within the codebase. Lemme give that a try

@Julien-R44 Julien-R44 marked this pull request as ready for review March 15, 2024 15:43
@McSneaky
Copy link

@Julien-R44 any change we can get this to v6 version? 😋 #25

@mrsafalpiya
Copy link

@Julien-R44 when can we expect this to be done? :)

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.

5 participants