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

fix(deps): revert to glob@7.2.0 for sequelize@6 & node@10 compatibility #1479

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

lukashroch
Copy link
Collaborator

reverting to glob@7.2.0 for sequelize@6 & node@10 compatibility

new PR as GH didn't allow me to revert dependabots PR, not sure why :-/

@WikiRik
Copy link
Member

WikiRik commented Oct 17, 2022

The CI didn't pick this up before, should we do something about that? Add a test for the following function (the only usage of glob) maybe?

/**
* Determines models from value
*/
export function getModels(arg: (ModelCtor | string)[], modelMatch: ModelMatch): ModelCtor[] {
const hasSupportedExtension = (path) => ['.ts', '.js'].indexOf(extname(path)) !== -1;
if (arg && typeof arg[0] === 'string') {
return arg.reduce((models: any[], dir) => {
if (!glob.hasMagic(dir) && !hasSupportedExtension(dir)) dir = join(dir as string, '/*');
const _models = glob
.sync(dir as string)
.filter(isImportable)
.map(getFullfilepathWithoutExtension)
.filter(uniqueFilter)
.map((fullPath) => {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const module = require(fullPath);
const fileName = basename(fullPath);
const matchedMemberKey = Object.keys(module).find((m) => modelMatch(fileName, m));
const matchedMember = matchedMemberKey ? module[matchedMemberKey] : undefined;
if (!matchedMember && !module.default) {
throw new Error(
`No default export defined for file "${fileName}" or ` +
`export does not satisfy filename.`
);
}
return matchedMember || module.default;
});
models.push(..._models);
return models;
}, []);
}
return arg as ModelCtor[];
}

Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

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

I also wouldn't recommend reverting the Dependabot PR directly since more changes have occured to the package-lock file. Better to do this from scratch as you did now

@lukashroch lukashroch merged commit 7c8eea7 into sequelize:master Oct 17, 2022
@lukashroch
Copy link
Collaborator Author

CI should probably have win platform, I think it's only ubuntu now. That would have picked 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.

None yet

2 participants