i18next / i18next-localstorage-backend

This is a i18next cache layer to be used in the browser. It will load and cache resources from localStorage and can be used in combination with the chained backend.
MIT License
88 stars 43 forks source link

Fixing a TypeScript error complaining of missing default export #8

Closed derek-pavao closed 5 years ago

derek-pavao commented 5 years ago

Was getting the following error in my TypeScript 2.7.2 project

 error TS1192: Module '"/Users/dpavao/Projects/web2/node_modules/i18next-localstorage-backend/index"' has no default export.

The following PR fixes that error

jamuhl commented 5 years ago

@VincentLanglet @rosskevin -> could this be merged

VincentLanglet commented 5 years ago

@dotDeeka @rosskevin It's weird.

In my recent project I use :

import i18next from 'i18next';
import LanguageDetector from 'i18next-browser-languagedetector';
import Backend from 'i18next-xhr-backend';
import { initReactI18next } from 'react-i18next';

export default i18next
  .use(Backend)
  .use(LanguageDetector)
  .use(initReactI18next)
  .init({ ...  });

With no error.

And if you look at the definition of browser-languagedetector here or xhr-backend here, you can see namespace, class and module are defined the same way than here. There is also chained-backend here.

VincentLanglet commented 5 years ago

I just tried yarn add i18next-localstorage-backend.

import i18next from 'i18next';
import LanguageDetector from 'i18next-browser-languagedetector';
import Backend from 'i18next-localstorage-backend';
import { initReactI18next } from 'react-i18next';

export default i18next
  .use(Backend)
  .use(LanguageDetector)
  .use(initReactI18next)
  .init({ ...  });

With no typescript error.

Edit: ok, I had these option in my tsconfig.json

"module": "esnext",
"moduleResolution": "node",
"allowSyntheticDefaultImports": true,
"esModuleInterop": true,
rosskevin commented 5 years ago

The PR is correct because the js code has a default export. TS users will see different behavior in userland depending on their allowSyntheticDefaultImports https://www.typescriptlang.org/docs/handbook/compiler-options.html

VincentLanglet commented 5 years ago

@rosskevin Indeed. We/I need to do the same for at least

I won't do this this night, but if not done, I'll try to take time for it tomorrow.

derek-pavao commented 5 years ago

@VincentLanglet I thought I had allowSyntheticImports: true in my tsconfig file, but apparently not. Thanks for making me look. 👍

jamuhl commented 5 years ago

published in i18next-localstorage-backend@2.1.1

VincentLanglet commented 5 years ago

@jamuhl @dotDeeka @rosskevin I still have doubt in this PR

Look at https://github.com/DefinitelyTyped/DefinitelyTyped/issues/33097#issuecomment-464127582

A true default export is

module.exports.default =

Here we have

module.exports =

I think we should revert this PR, since there si no default export.

derek-pavao commented 5 years ago

@VincentLanglet, so if you guys reverted this, what would be my solution? To just allowSyntheticImports: true ?

VincentLanglet commented 5 years ago

Yes

Or use import * as Backend from ...

jamuhl commented 5 years ago

I think we got the same problem here as with i18next: https://github.com/i18next/i18next/pull/1204

There is a default export using the files from build -> /dist/es or `/dist/commonjs``

but not for node.js env (where this module is useless i would say) /index.js where it takes the default as main export module.exports = require('./dist/commonjs/index.js').default;

VincentLanglet commented 5 years ago

@jamuhl

Since the file index.js only export this way

module.exports = require('./dist/commonjs/index.js').default;

I think the good utilisation of this module is


Import * as Backend from ...
jamuhl commented 5 years ago

Not sure...it really depends on your environment -> /index.js is only pulled by node.js, while eg. webpack will pull https://github.com/i18next/i18next-localstorage-backend/blob/master/package.json#L7

No idea what typescript does but Import * as Backend from ... in Javascript world would be not correct.

VincentLanglet commented 5 years ago

I thought Import * as Backend from ... would work the same way Import * as React from 'react', I didn't know about the effect of the line "jsnext:main": "dist/es/index.js"

Maybe we have too to do the improvement

module.exports = main;
module.exports.default = main;

Don't really know, i'm not expert in module resolution...

VincentLanglet commented 5 years ago

@jamuhl I'll have these 3 PR to merge, after we're sure what we're doing.

https://github.com/i18next/i18next-browser-languageDetector/pull/165 https://github.com/i18next/i18next-xhr-backend/pull/298 https://github.com/i18next/i18next-chained-backend/pull/6

jamuhl commented 5 years ago

Yes...adding

module.exports = main;
module.exports.default = main;

would not hurt i guess