search

i18next / i18next-express-middleware

[deprecated] can be replaced with i18next-http-middleware
https://github.com/i18next/i18next-http-middleware
MIT License
206 stars 52 forks source link

fix: TypeScript i18next import #192

Closed marcobiedermann closed 5 years ago

marcobiedermann commented 5 years ago

Description

Fix TypeScript import of i18next in module declaration.

Since i18next does not provide a default export you have to import everything. Setting the TypeScript compile option esModuleInterop is not a good practise and not an option for most projects out there.

Error stack

node_modules/i18next-express-middleware/index.d.ts:2:8 - error TS1259: Module '"/Users/biedema/Documents/GitHub/work/moovel/mobility-benefits-core/node_modules/i18next/index"' can only be default-imported using the 'esModuleInterop' flag

2 import i18next from 'i18next';
         ~~~~~~~

  node_modules/i18next/index.d.ts:988:1
    988 export = i18next;
        ~~~~~~~~~~~~~~~~~
    This module is declared with using 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.
jamuhl commented 5 years ago

@rosskevin can you check this one?

rosskevin commented 5 years ago

This is correct and works for both variations on esModuleInterop, thank you @marcobiedermann. I wish there were tests in this project like i18next or react-i18next, but the original committer did not add any. They are welcome if you want to work on them.

@jamuhl please merge and release should be patch

marcobiedermann commented 5 years ago

@rosskevin

Unfortunately I do not see any way how to write tests for this by now since this project is written in vanilla JavaScript and I would not add a test written in TypeScript to be honest. If you really want to tests this, along with all other TypeScript definitions, I suggest to port this project over to TypeScript entirely.

Would be great if you guys could merge this bug and release a fix on npm. Currently I forked this project and included it into my main project

rosskevin commented 5 years ago

@marcobiedermann if you want to see how to write usage tests, check out how we do it in i18next https://github.com/i18next/i18next/blob/master/package.json#L84

@jamuhl doesn't want to use typescript directly. I do think it would be much simpler for all of this source to be in typescript, but it's jamuhl's prerogative and I respect that.

marcobiedermann commented 5 years ago

@rosskevin

Thank you very much for pointing out. Yes a quick test would be to run the TypeScript compiler without outputting any code. I did some quick testing and it seems like your TypeScript definitions do not match the current version of i18next. In general, all your dependencies are quite outdated. I suggest to update all to the most recent version and add TypeScript checking afterwards. Otherwise you run into too many compatibility issues.

rosskevin commented 5 years ago

Please update at least the minimum necessary i18next version in the package.json. Feel free to use ncu and update all.

PS I'm neither maintainer or user of this package, just helping out. I help maintain i18next ts primarily.

jamuhl commented 5 years ago

published in i18next-express-middleware@1.8.2