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

fix: unable to import module from typescript #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mleguen
Copy link

@mleguen mleguen commented Dec 3, 2019

Allow import in typescript:

import timestamp = require('timestamp');

EDIT: updated with typescript recommanded import syntax for commonjs module as recommanded by @fregante (see below).

@mleguen
Copy link
Author

mleguen commented Dec 3, 2019

Close #14

@fregante
Copy link

fregante commented Dec 4, 2019

That * as timestamp should never be used. Either use import or require

@mleguen
Copy link
Author

mleguen commented Dec 4, 2019

@fregante Could you be a little more explicit about why it should never be used?

Isn't it the standard typescript way to access a commonjs module's module.export (as in typescript docs])?

@fregante
Copy link

fregante commented Dec 4, 2019

That page is for ES Modules, you can use import * as name because ES Modules have multiple exports. Common JS modules only have one import and you should only use require (unless you have the compatibility option enabled). There’s an additional way to require them but I don’t remember exactly when it was used:

import time = require('time')

I think it’s just an old way though

@papb
Copy link

papb commented Dec 4, 2019

import time = require('time')

Interestingly, recently I've seen this form of importing being preferred over all others, see here and here.

@mleguen
Copy link
Author

mleguen commented Dec 10, 2019

I did not look at the right part of the typescript documentation. Thank you @fregante for pointing this.

The recommanded syntax for commonjs module is indeed:

import time = require('time')

I edited the issue description and fixed the README in the PR.

@luckydrq
Copy link

I did not look at the right part of the typescript documentation. Thank you @fregante for pointing this.

The recommanded syntax for commonjs module is indeed:

import time = require('time')

I edited the issue description and fixed the README in the PR.

This is still not corrected because tsc would compile it as follow:

const time_stamp_1 = require("time-stamp");
const dateStr = time_stamp_1.default('YYYY-MM-DD', new Date(time));

Since exports.default is not defined, the code would fail.

@fregante
Copy link

fregante commented Nov 24, 2020

This is still not corrected because tsc would compile it as follow:

That's not right. Demo with module: 'commonjs'

import something = require('something') still uses the regular CommonJS pattern and is completely unrelated to ES Modules.

Documentation: https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require

@luckydrq
Copy link

luckydrq commented Dec 11, 2020

So is this PR ready for merge?

@fregante
Copy link

The only thing to notice is that this is a breaking change for TypeScript users that aren’t using esModuleInteropt since import from now has to be import = require

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.

None yet

4 participants