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 numeral import Typescript error #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shalomscott
Copy link
Contributor

Small Typescript error. However this resulted in significant noise for my project when bundling with Rollup. Output was:

src/main.ts → public/build/bundle.js...
(!) Cannot call a namespace ('numeral')
node_modules/kosher-zmanim/dist/lib-es6/util/ZmanimFormatter.js: (169:18)
167:             return ZmanimFormatter.formatXSDDurationTime(time);
168:         }
169:         let sb = (numeral(time.getHours()).format(this.hourNF)).concat(':')
                       ^
170:             .concat(numeral(time.getMinutes()).format(ZmanimFormatter.minuteSecondNF).toString());
171:         if (this.useSeconds) {
(!) Cannot call a namespace ('numeral')
node_modules/kosher-zmanim/dist/lib-es6/util/ZmanimFormatter.js: (170:20)
168:         }
169:         let sb = (numeral(time.getHours()).format(this.hourNF)).concat(':')
170:             .concat(numeral(time.getMinutes()).format(ZmanimFormatter.minuteSecondNF).toString());
                         ^
171:         if (this.useSeconds) {
172:             sb = sb.concat(':').concat(numeral(time.getSeconds()).format(ZmanimFormatter.minuteSecondNF).toString());
...

... and so on.

Related discussion here: Cannot call a namespace ('express').

After this tiny fix the noise was gone.

@BehindTheMath
Copy link
Owner

BehindTheMath commented Jan 19, 2021

Did you run the unit tests? I need to set up an Action to run them automatically, but when I made this change and ran them, I got the following error for many tests:

TypeError: numeral_1.default is not a function

    at Function.formatDecimal (C:\Users\aryeh\WebstormProjects\KosherZmanim\src\util\ZmanimFormatter.ts:381:64)
    at Function.getOutputMetadata (C:\Users\aryeh\WebstormProjects\KosherZmanim\src\util\ZmanimFormatter.ts:507:34)
    at Function.toJSON (C:\Users\aryeh\WebstormProjects\KosherZmanim\src\util\ZmanimFormatter.ts:477:33)
    at Object.getZmanimJson (C:\Users\aryeh\WebstormProjects\KosherZmanim\src\kosher-zmanim.ts:16:26)
    at Object.<anonymous> (C:\Users\aryeh\WebstormProjects\KosherZmanim\tests\kosher-zmanim.test.ts:21:35)
    at Object.asyncJestTest (C:\Users\aryeh\WebstormProjects\KosherZmanim\node_modules\jest-jasmine2\build\jasmineAsyncInstall.js:106:37)
    at C:\Users\aryeh\WebstormProjects\KosherZmanim\node_modules\jest-jasmine2\build\queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (C:\Users\aryeh\WebstormProjects\KosherZmanim\node_modules\jest-jasmine2\build\queueRunner.js:28:19)
    at C:\Users\aryeh\WebstormProjects\KosherZmanim\node_modules\jest-jasmine2\build\queueRunner.js:75:41
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

@shalomscott
Copy link
Contributor Author

My bad, I had not run them.

After looking at numeral.js's type definitions and noticing the export = numeral line, I was able to come to a section in the TS docs where it say the proper way to import such an object is with the following interesting syntax: import module = require("module"). However trying this in Kosher Zmanim broke the build. Got the following:

src/util/ZmanimFormatter.ts:2:1 - error TS1202: Import assignment cannot be used when targeting ECMAScript modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.

2 import numeral = require("numeral");
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In the end, managed to skirt the Typescript issue with this unfortunate bit of syntax:

import * as numeral_ from 'numeral';
const numeral = numeral_;

Inspiration from here: jvandemo/generator-angular2-library#221 (comment)

Updated PR incoming.

@shalomscott
Copy link
Contributor Author

keepalive

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.

2 participants