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

Typescript 4.5.4 does not like sequelize-ESM imports in v6.12 #13791

Closed
2 tasks done
dilyanpalauzov opened this issue Dec 18, 2021 · 21 comments · Fixed by #13731, #13797 or #13805
Closed
2 tasks done

Typescript 4.5.4 does not like sequelize-ESM imports in v6.12 #13791

dilyanpalauzov opened this issue Dec 18, 2021 · 21 comments · Fixed by #13731, #13797 or #13805
Assignees
Labels

Comments

@dilyanpalauzov
Copy link
Contributor

dilyanpalauzov commented Dec 18, 2021

My understanding of #13689, included in v6.12 is, that sequelize is not any more exported just in CommonJS, but can be used also as ESM-imports. Thus, the code in v6.11

import sequelize_ from 'sequelize'
const {DataTypes, Model, Op} = sequelize_ //this is CommonJS hack

can be changed in 6.12 to import {DataTypes, Model, Op} from 'sequelize'.

Typescript 4.5.4 fails on the above code with

$ node_modules/.bin/tsc
node_modules/sequelize/lib/operators.ts:524:1 - error TS1203: Export assignment cannot be used when targeting ECMAScript modules. Consider using 'export default' or another module format instead.

524 export = Op;
    ~~~~~~~~~~~~

Found 1 error.

My understanding is, that Typescript is only supposed to verify the types in sequelize by parsing the code in the node_modules/sequelize/types directory, and not loose time to verify each file in node_modules/sequelize/lib/. But types/index.d.ts contains import * as Op from "../lib/operators"; so my assumption is not correct. Likewise for node_modules/sequelize/types/lib/model.d.ts:import * as Op from '../../lib/operators';

Bug Report Checklist

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.

Would you be willing to resolve this issue by submitting a Pull Request?

  • No, I don't have the time and I wouldn't even know how to start.
@WikiRik
Copy link
Member

WikiRik commented Dec 18, 2021

@ephys can you take a look at this?

@fzn0x fzn0x linked a pull request Dec 18, 2021 that will close this issue
7 tasks
@shoeyn
Copy link

shoeyn commented Dec 19, 2021

I'm having this issue also since the 6.12 release

Changing export = Op; to export default Op;
Then changing cjs imports from require('./operators') to require('./operators').default
and esm imports from import * as Op to import Op
seems to pass typescript tests, but I don't know if there's any other consequences and whether this sort of change conforms with your code style.

@ephys
Copy link
Member

ephys commented Dec 20, 2021

@WikiRik Yep! I'll look into this :)

@ephys
Copy link
Member

ephys commented Dec 21, 2021

Release 6.12.1 should fix this as we've reverted to the previous import style.

If it does not, my recommendation would be to use the skipLibCheck option until we migrate to esm import/exports in v7

@shoeyn
Copy link

shoeyn commented Dec 21, 2021

Still getting the error on 6.12.1 even with skipLibCheck, I'll just revert to 6.11 for now and wait for the full v7 release

@ephys
Copy link
Member

ephys commented Dec 21, 2021

@shoeyn can you post your tsconfig.json?

@shoeyn
Copy link

shoeyn commented Dec 21, 2021

{
"compilerOptions": {
"target": "es2019",
"module": "es2020",
"strict": true,
"noImplicitAny": true,
"strictNullChecks": true,
"strictFunctionTypes": true,
"strictBindCallApply": true,
"strictPropertyInitialization": true,
"noImplicitThis": true,
"alwaysStrict": true,
"noUnusedLocals": true,
"noUnusedParameters": false,
"noFallthroughCasesInSwitch": true,
"jsx": "preserve",
"importHelpers": true,
"moduleResolution": "node",
"allowJs": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": true,
"forceConsistentCasingInFileNames": true,
"pretty": true,
"sourceMap": true,
"baseUrl": ".",
"types": ["webpack-env"],
"lib": ["esnext", "dom", "DOM.Iterable"]
},
}

// edit: This is after removing skiplibcheck again and removing all sorts of paths
I've gone through line by line and simply just having the target set as es* is what causes the error

@Papa2Carlro
Copy link

I have the same problem.
Version 6.12.1 did not resolve this issue. Locally, I used version 6.6.*, And everything worked for me

On the server, my project compiles the Docker and it uses the latest version. Because of this, I can't unload a very important fix

In my tsconfig.json skipLibCheck is true

Locally, I solved this problem like this:
// @ts-ignore
export = Op;

For some reason I can't use the previous version, the latest one is being downloaded
What can I do to prevent it from breaking my entire site?

@shoeyn
Copy link

shoeyn commented Dec 21, 2021

@Papa2Carlro In your package.json make sure to have sequelize as:
"sequelize": "6.11.0"
without any characters in front of the version number such as: ^ > ~ or the word latest

@ephys
Copy link
Member

ephys commented Dec 21, 2021

This is annoying.

  • TypeScript forbids using export = when the user project declares using module (ignoring sequelize's own tsconfig.json, and even though Node ESM can import CommonJS modules).
  • module.exports is a no go because we lose all typing information
  • moving to export default will cause a breaking change to users incorrectly doing const Op = require('sequelize/lib/operators') instead of const { Op } = require('sequelize'): The transpiled export is module.exports.default = Op instead of the previous module.exports = Op.

Although, If I do

export default Op;
module.exports = Op;

it works (for as long as its transpiled to commonjs).

@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 6.12.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sdepold
Copy link
Member

sdepold commented Dec 22, 2021

Can you give it a try and reopen if still an issue?

@dilyanpalauzov
Copy link
Contributor Author

Now it works.

Github does not allow the reporter to reopen issues, which were not closed by the reporter.

@sdepold
Copy link
Member

sdepold commented Dec 22, 2021

ah :) but all good?

@dilyanpalauzov
Copy link
Contributor Author

Now it works.

@joaoe
Copy link
Contributor

joaoe commented Dec 26, 2021

Hi.
I'm running node.js 16 and have a checkout of the latest commit from the main branch.

I'm debugging unit tests that were failing due to a "@@" operator, and I see the whole QueryGenerator.OperatorMap is reduced to {undefined:"@@"}. In lib/dialects/abstract/query-generator.js the line const Op = require('../../../operators'); returns an object with a default property instead of all the expected operators.
The generated js for operators.ts shows this

var operators_exports = {};
__export(operators_exports, {
  default: () => operators_default
});
// ...
module.exports = Op;
module.exports = __toCommonJS(operators_exports);

Not sure if this is expected.

@ephys
Copy link
Member

ephys commented Dec 27, 2021

Just ran build.js and the output for operators.ts (/dist/lib/operators.js) looks like this (which is the intended output):

Screenshot 2021-12-27 at 16 19 11

What steps did you take to get your output?

@joaoe
Copy link
Contributor

joaoe commented Dec 27, 2021

Hi @ephys
I found the problem.
I checked out the tip of the main branch and ran npm install.
npm looks at package.json, namely esbuild@^0.14.1, and installs whatever latest version is after 0.14.1 which currently is 0.14.8.
I recalled that yarn seems to be what you are using, so I ran yarn install. that one looks at yarn.lock which has version 0.14.1 locked in.

Version0.14.1 of esbuild does not show the problem, version 0.14.8 does.
So, I think you might want to lock esbuild to version 0.14.1 in package.json until you remove the workaround in operators.js.

@ephys
Copy link
Member

ephys commented Dec 28, 2021

Good catch, we'll do that :)

@ephys
Copy link
Member

ephys commented Dec 29, 2021

esbuild version has been locked https://github.com/sequelize/sequelize/pull/13836/files

@WikiRik
Copy link
Member

WikiRik commented Dec 29, 2021

esbuild version has been locked https://github.com/sequelize/sequelize/pull/13836/files

This is also part of the latest release (v6.12.4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment