Skip to content

Conversation

p3drosola
Copy link

@p3drosola p3drosola commented Jul 16, 2025

πŸ”— Linked issue

https://discord.com/channels/423256550564691970/1395127978777772052

Docs update: adonisjs/lucid.adonisjs.com#58

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Impossible to use other model attributes in consume function.

The consume function signature is (value, attribute ,model) =>

The problem is that since the attributes are parsed and assigned to the model in a random order you can't assume that a certain other attribute has already been assigned to the model, so you can't use other model attributes when consuming an attribute.

I read the lucid source code and there doesn't seem to be any way to access the adapterResult from the consume hook, since it's not a property of the model and only exists in the scope.

This seems to be the cleanest way to implement this functionality, since it's backwards compatible

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests
  • I have updated the documentation accordingly.

@p3drosola p3drosola force-pushed the lucid-column-consume-adapter-result branch from 80c3e8a to 73b3853 Compare July 17, 2025 00:31
@p3drosola
Copy link
Author

@thetutlage Does this explanation make sense?

@thetutlage
Copy link
Member

Hey, can you please also open another PR for the docs? And add some tests as well

@p3drosola
Copy link
Author

Where would you like to see the docs?

This is the most relevant section I can find, but it doesn't contain the existing extra arguments (attribute, model). It seems like the sort of thing left for Typescript types to explain.

https://lucid.adonisjs.com/docs/models#preparing-and-consuming-columns

@p3drosola
Copy link
Author

Added test coverage

@p3drosola
Copy link
Author

bumping this PR. tests are passing. LMK if/where you would like to see docs

@thetutlage
Copy link
Member

This is the most relevant section I can find, but it doesn't contain the existing extra arguments (attribute, model). It seems like the sort of thing left for Typescript types to explain.

Yup, let's add it there itself

@thetutlage
Copy link
Member

Can you please run npm run format to fix the linting errors

@p3drosola
Copy link
Author

Hi @thetutlage

I ran the formatter and created a new PR for the docs. adonisjs/lucid.adonisjs.com#58

Does this look ok? Are we good to merge this?

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.

2 participants