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
feat: gen /lib & /types from /src & drop /dist (v6) #14063
Conversation
I accidentally ran eslint on .d.ts files it looks like. I can revert those changes if you want. Especially since they go against the new lint config Edit: i'll revert the changes that go against the current linter but keep the others - as I'll cherry pick this PR in main afterwards |
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.
Two small remarks but looks good. Too bad we can't really do a v6-beta release to let people test this out, so we'll just have to make a new release with this. Should we rename the PR to be a feature?
Since I prefer src over lib anyway I'm a big fan if this change 😅 |
Shouldn't we have the same in v7? |
I plan on opening a follow up PR to cherry pick in main (which I assume we can easily merge in v7 too? I don't know the extend to which main and v7 differ). Starting with v6 felt safer for some reason
@WikiRik I agree, making this semver-minor would be better
I've actually managed to reproduce the reported issue in a project of mine. I'll double-check that it's gone. Edit: Well look at that, my own code was importing things from a wildly incorrect place :/ Likely due to webstorm auto-importing incorrectly. I'll open a follow-up PR to the follow-up PR to block users from importing any other file than 'sequelize' and 'sequelize/package.json' |
Oh v7 is just a copy of main. So if we target main, we can eventually just merge it into v7 |
Here we go. I hope this is the end of this painful situation :/ |
🎉 This PR is included in version 6.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Conflicts: .eslintrc.json .gitignore build.js lib/dialects/abstract/index.js lib/index-hints.js lib/index-hints.ts lib/query-types.js lib/query-types.ts lib/table-hints.js lib/table-hints.ts lib/transaction.js lib/transaction.ts package.json src/dialects/abstract/index.js src/dialects/abstract/query-interface.d.ts src/dialects/abstract/query.d.ts src/index-hints.d.ts src/index-hints.js src/query-types.d.ts src/query-types.js src/sequelize.d.ts src/table-hints.d.ts src/table-hints.js src/transaction.d.ts src/transaction.js test/support.js test/tsconfig.json test/types/e2e/docs-example.ts types/index.d.ts types/lib/index-hints.d.ts types/lib/query-interface.d.ts types/lib/query-types.d.ts types/lib/query.d.ts types/lib/sequelize.d.ts types/lib/table-hints.d.ts types/lib/transaction.d.ts types/test/e2e/docs-example.ts types/test/tsconfig.json types/tsconfig.json
Closes #14038
This is a pretty big PR that changes how we build sequelize.
Hopefully this is the last post TS migration fix PR.
During the initial TS migration,
/lib
was changed from a pure JS folder to a mix of JS and TS./dist/lib
became the "new"/lib
.This already broke things so we told node to map
/lib
to/dist/lib
using 'exports' in package.json.We still had issues where users would sometimes manage to import their typings from
/lib
instead of/types
so we deduplicated typings to ensure users would always import the right typing file.This lead to a subsequent issue where, because files in
/lib
were typescript files and not typescript declaration files, typescript would complain if the code did not match how the user configured theirtsconfig.json
So now, I'm opening a pretty drastic PR: The goal is to allow .d.ts to reference our new .ts files without having to publish any .ts file.
/lib
,/types
have been merged together in/src
/lib
and/types
are generated usingyarn build
. They contain respectively only pure JS and the ts declarations. This matches as closely as I could what we had before the TS migration./dist
is gone(which should be fine because it was already impossible to import directly due toexports
in package.json. It had to be imported as "sequelize/lib")/lib
, and/types
folders are published to npmexports
in package.json reflects the changes so imports should not be broken.This PR targets v6.
Unfortunately, this is going to break many/all PRs once added to main. :(
I'll open a PR to do the same in main once this one is merged.