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

omitHook #14053

Open
njavilas2015 opened this issue Feb 4, 2022 · 6 comments
Open

omitHook #14053

njavilas2015 opened this issue Feb 4, 2022 · 6 comments
Labels
type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@njavilas2015
Copy link

njavilas2015 commented Feb 4, 2022

good morning I find myself working with this incredible library in my microservice system and I observe that there is no way to omit a universal hook. There is a possibility that you might consider adding an option to skip a specific hook I will give a very used use case in my little service

the hooks works fine when I call the destroy function
example controller

export const Delete = async (options) => {
    try {

       await Model.destroy(options);

         ......

    } catch (error) {

       console.log(error)

    }
};

Model example

    export const Model = sequelize.define(name, {
        pk: {
            type: S.DataTypes.UUID, primaryKey: true, unique: true,
            defaultValue: S.DataTypes.UUIDV4
        },
    
       /* fields**/
    
    }, {
        timestamps: true, freezeTableName: true,
        hooks: {
            beforeCreate, beforeDestroy, beforeUpdate, beforeBulkCreate,
    
            beforeBulkDestroy, beforeBulkUpdate
        }
    });
export const DeletePer = async (options) => {
    try {

        let instance = await Model.destroy({where: { field: options.field }, omitHookDestroy: true   }); <--- New fixture

        return  instance;

    } catch (error) {

       console.log(error)

    }
};

I would like only in that function to avoid the destroy hooks

@manojmula
Copy link

I have found a stackoverlow on similar question , see if it can help you , or else please let me know.here is the link : https://stackoverflow.com/questions/20707199/how-to-ignore-hooks

@ephys
Copy link
Member

ephys commented Feb 11, 2022

Can't promise this will be implemented, your best shot would be to open a PR

If this were to be implemented, I think an API that mimics attributes would be nice:

// this already exists today
Model.destroy({ hooks: false  });

// only run specific hooks
Model.destroy({ hooks: ['beforeDestroy']  });

// exclude some hooks
Model.destroy({ hooks: { exclude: ['beforeDestroy'] }  });

@ephys ephys added the type: feature For issues and PRs. For new features. Never breaking changes. label Feb 11, 2022
@WikiRik
Copy link
Member

WikiRik commented Feb 11, 2022

Agree with the API as proposed by @ephys
In my opinion this would be a nice addition, if somebody is willing to work on this

@njavilas2015
Copy link
Author

njavilas2015 commented Feb 11, 2022

image
check the destroy method options api but the hooks field is a boolean
image

I would love to help! but I still don't have much experience, I was inspecting the code and it seems very complicated

@ephys
Copy link
Member

ephys commented Feb 16, 2022

The change is not too hard but a lot of code needs to be changed in many different files.
The "runHooks" function is located here https://github.com/sequelize/sequelize/blob/main/src/hooks.js#L94

But the code deciding whether the hook should or shouldn't run is duplicated in many places, such as here: https://github.com/sequelize/sequelize/blob/main/src/model.js#L1813

Best solution IMO would be to always pass an option bag to runHooks, then runHooks decides whether the hook will run based on options.hooks

The typings would also need to be updated here: https://github.com/sequelize/sequelize/blob/main/src/model.d.ts#L750

We'd need to write an extensive amount of tests to ensure the option works in each place, which is the really painful part

@njavilas2015
Copy link
Author

my respects, it's an incredible library but I got lost very easily jeje ​​I've been trying to understand the code for 2 months and I have experience in js you get lost in so much code haha, consult us it would be better to start separating by files as much as possible for the debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests

4 participants