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: fix invalid ts import style of lib/operators #13797
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I did make a suggested change but it's inconsistent throughout the files either way si just see what you do with it
I don't think the failed test is related to this PR. Very strange. Can we try relaunching that test? Edit: yep, passed now :) |
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
v6.12.1 is on it's way in this very moment :) |
🤞 |
🎉 This PR is included in version 6.12.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* fix: fix invalid ts import style of lib/operators * refactor: align types/index.d.ts Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com> Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Pull Request Checklist
Please make sure to review and check all of these items:
npm run test
ornpm run test-DIALECT
pass with this change (including linting)?Description Of Change
Replacement PR of #13796
Attempt to fix #13791
There was an issue in PR #13731 where
Op
was exported usingexport =
but was imported usingimport Op from './operator'
which is not compatible (source https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require).This is a simple fix that changes the imports back to
import Op = require('./lib/operators')
syntax.This one does not include any lint correction either as types/* is not linted and will be removed anyway (#13796 (comment))
Concerns
Things I noticed while working on this PR. Putting them here to decide between fixing them here or opening an issue.
Avoiding breaking changes
As talked about in #13796, we should stick to this import/export syntax in v6 to avoid breaking changes.
We can revisit this in v7.
Two operators.ts definitions
index.d.ts
imports'../lib/operators'
which points tosequelize/lib/operators.ts
instead ofsequelize/dist/lib/operators.d.ts
.This could be a problem.
A potential solution to import /lib from /types would be to change the import from
../lib/operators
tosequelize/lib/operators
.