Skip to content

Use esModuleInterop and fix corresponding issues #360

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

Conversation

johannes-scharlach
Copy link
Contributor

@johannes-scharlach johannes-scharlach commented Apr 24, 2018

Fixes #330

I've also tried bumping typescript and setting esModuleInterop to true. The proposed changes also worked with the newer version, but I'm not sure if that would introduce a breaking change to sequelize-typescript. The recommendation seems to be to set that flag to true when you can.

I've taken the solution from this issue microsoft/TypeScript#21535 (comment)

@codecov-io
Copy link

Codecov Report

Merging #360 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #360   +/-   ##
=======================================
  Coverage   96.66%   96.66%           
=======================================
  Files         117      117           
  Lines        1019     1019           
  Branches      133      133           
=======================================
  Hits          985      985           
  Misses         15       15           
  Partials       19       19
Impacted Files Coverage Δ
lib/models/v3/Sequelize.ts 98.27% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64586e6...426384f. Read the comment docs.

@RobinBuschmann
Copy link
Member

Ha, I never thought of using = require() syntax here, but it does the job very well. Thanks for implementing!

@RobinBuschmann RobinBuschmann merged commit 13d74e6 into sequelize:master Apr 24, 2018
@RobinBuschmann
Copy link
Member

I just published a new release including this change: [email protected]

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.

3 participants