palantir / blueprint

A React-based UI toolkit for the web
https://blueprintjs.com/
Apache License 2.0
20.75k stars 2.18k forks source link

TypeError: moment is not a function #959

Closed zsherman closed 6 years ago

zsherman commented 7 years ago

It seems that moment@2.14 exports moment as a function by default but it's being imported with the syntax import * as moment from 'moment', yet this file https://github.com/palantir/blueprint/blob/master/packages/datetime/src/common/dateUtils.ts #calls moment as a function in multiple places, which leads to this error. Has anyone seen this or been able to reproduce? Might be missing something here...

Also obligatory shoutout for the awesome lib, thanks for all your hard work.

Bug report

Steps to reproduce

  1. Import DateRangeInput and pass in valid dates

Actual behavior

TypeError: moment is not a function is thrown

Expected behavior

Should not throw an error.

giladgray commented 7 years ago

@zsherman hmm nothing seems to have changed in moment itself--usage has always been as a function. are you sure it's installed and being imported correctly in your application?

adidahiya commented 7 years ago

Do you mean to say that moment is exported as a default export (export default moment)? That gets compiled down to a simple module.exports in commonJS. The source here suggests that a function is the module export: https://unpkg.com/moment@2.14.1/moment.js

Anything interesting about your build system that may cause moment to not be loaded or bundled correctly?

zsherman commented 7 years ago

Hmm don't think I'm doing anything special, when I inspect moment in the chrome debugger I'm seeing that it is indeed an object, not a function:

screenshot 2017-04-06 18 54 27

zsherman commented 7 years ago

For what it's worth, I have no issues with the DayPicker, only with anything that relies on dateInput.

zsherman commented 7 years ago

My guess is it's the way it's being imported here https://github.com/palantir/blueprint/blob/master/packages/datetime/src/common/dateUtils.ts#L8, which I thought pulls the export into a new object, which I can reproduce locally. Importing like import moment from 'moment' pulls in a function.

adidahiya commented 7 years ago

which I thought pulls the export into a new object

not quite; import * as moment assigns module.exports of the entry point of the package to the moment symbol in the scope. module.exports is often an object literal, but it can be any value and in the case of the moment library it is a function.

can you provide a minimal repro of your error? what build system or module loader are you using? where exactly are you stepping through code to get moment to show up like that in the chrome debugger? I don't think there's anything wrong with the way @blueprintjs/datetime imports moment; you'll notice that most TS code on github does it this way.

zsherman commented 7 years ago

thanks @adidahiya, so here's the webpack config my app is based off of https://github.com/react-boilerplate/react-boilerplate/blob/master/package.json, I was able to run into the same error with a fresh install so I'm suspicious that it's something to do with the way modules are being imported there.

In order to inspect the value of moment I'm just placing a breakpoint at the line where it's failing and added adding a watcher here https://www.dropbox.com/s/jbv22lp0bpaxbxh/Screenshot%202017-04-07%2010.10.03.png?dl=0.

zsherman commented 7 years ago

@adidahiya here's an example repo with a fresh install of blueprint, if you installs deps and load up the homepage you should see the error https://github.com/zsherman/blueprint-test

tannerlinsley commented 7 years ago

I can confirm this. When importing with * syntax, I indeed receive an object with moment as a key. This is in line with how moment is being exported as "export default moment" here: https://github.com/moment/moment/blob/develop/src/moment.js#L82

When importing as "import moment from 'moment'" only then do I receive a function.

tannerlinsley commented 7 years ago

Also worth noting that moments module.exports is not a function, but as per es6 module export spec, it is an object with moment assigned to the default key.

giladgray commented 7 years ago

this seems like an actual issue with moment & typescript: https://github.com/moment/moment/issues/3650

they clearly use ES6 export default in the main file, but at the same time our import * as moment has worked swimmingly for months. seems to be an issue related to how you run the code in the browser.

seansfkelley commented 7 years ago

The source of moment uses export default, but the transpiled code ends up as module.exports = ..., meaning that the obnoxious export = ... syntax in the typings is correct, despite what the original source would have you believe, and import * as moment from 'moment' is correct as long as you aren't using allowSyntheticDefaultImports.

opavader commented 7 years ago

having the exact same issues with datetime@1.14.0 & moment@2.18.1. Any possibles workaround or idea for temporary fixing this?

seansfkelley commented 7 years ago

The root cause is probably an unfortunate combination of compiler flags, possibly across languages/compilers, causing a disconnect between what you think you're writing and what is actually coming out the other end.

@opavader / @zsherman are you using Typescript or plain Javascript? Babel also does some weird stuff around these "synthetic defaults" (which I think causes more problems than it solves), so I'd poke around the transpiled output of your project, Blueprint and your version of moment to see which ones are in agreement and which are importing the wrong thing.

bitIO commented 7 years ago

I'm also using reactboilerplate and I have the same issue. Fixed adding the package moment-es6.

opavader commented 7 years ago

I@seansfkelley Using typescript. I downgraded my moment version to 2.14.1 but even then same error. Setting allowSyntheticDefaultImports to false will break other packages in use. My compiler options in tsconfig.json is

"compilerOptions": {
        "noUnusedParameters": true,
        "noUnusedLocals": true,
        "skipLibCheck": true,
        "noEmit": true,
        "target": "es6",
        "module": "commonjs",
        "noImplicitAny": false,
        "sourceMap": true,
        "inlineSourceMap": false,
        "inlineSources": false,
        "preserveConstEnums": true,
        "moduleResolution": "node",
        "experimentalDecorators": true,
        "jsx": "preserve",
        "lib": [
            "dom",
            "es5",
            "es6",
            "es7",
            "es2017.object"
        ],
        "allowSyntheticDefaultImports": true,
        "baseUrl": "./",
        "paths": {
            "ArchApp/*": [
                "src/*"
            ]
        }
    },
wshaver commented 7 years ago

Super hacky, but this is how I solved my problem with moment/typescript/webpack:

let moment = require("moment");
if ("default" in moment) {
    moment = moment["default"];
}
opavader commented 7 years ago

Yeah, I did the same. Wierd that even https://github.com/moment/moment-timezone/issues/449 has the same issue.

blackyale commented 6 years ago

@wshaver Your solution works for me, I had extra problems because I was creating a library with ng-packagr and it works now. Thanks.

monikaja commented 6 years ago

Word "require" makes my ionic app execution fail (ts error) with @wshaver solution.

giladgray commented 6 years ago

2.0.0-rc.3 now uses Typescript's new esModuleInterop and the latest version of tslib, which should fix any moment import issues.

additionally, datetime@2.0 no longer depends on moment so that should further solve this problem. (timezone still uses moment-timezone though)

minato-namikaze commented 6 years ago

@seansfkelley thanks. You saved my weekend :-)

angelarted commented 5 years ago

So definitely, to get moment work as a function, if you are using ES6 and babel, you must import it in this way: import moment from 'moment' and not, as written in the documentation import * as moment from 'moment'

GeorgeWL commented 5 years ago

This isn't resolved though really, as tests should be agnostic and not force you to have to change how you import into modules for them to work.

Can we re-open this please?

cherucole commented 4 years ago

import moment from 'moment'

the solution import moment from 'moment' worked for me

hlawuleka commented 4 years ago

My guess is it's the way it's being imported here https://github.com/palantir/blueprint/blob/master/packages/datetime/src/common/dateUtils.ts#L8, which I thought pulls the export into a new object, which I can reproduce locally. Importing like import moment from 'moment' pulls in a function.

Consider import moment, * as MyObjectMoment from 'moment';

estrings commented 4 years ago

So definitely, to get moment work as a function, if you are using ES6 and babel, you must import it in this way: import moment from 'moment' and not, as written in the documentation import * as moment from 'moment'

you are doing well dear

GeorgeWL commented 4 years ago

This isn't resolved though really, as tests should be agnostic and not force you to have to change how you import into modules for them to work.

Can we re-open this please?

ihor-zinchenko commented 4 years ago

This module is declared with using 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

Ritin commented 4 years ago

import * as temp from 'moment'; const moment = temp["default"];

GeorgeWL commented 4 years ago

that's bad practice though to redefine imports.

ihor-zinchenko commented 4 years ago

import moment from 'moment'; for me

This module is declared with using 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

adidahiya commented 4 years ago

This isn't resolved though really, as tests should be agnostic and not force you to have to change how you import into modules for them to work.

Which part of this issue is specific to tests?

This is an old issue, but @seansfkelley's comment https://github.com/palantir/blueprint/issues/959#issuecomment-295346290 is still essentially correct (albeit with updated code links: moment default export source, moment transpiled export, moment export type definition). So I don't think this requires a change in Blueprint. This issue can be resolved by tweaking your build tooling. If you can demonstrate a clear repro of a bug that warrants a change in Blueprint, please file a new issue and link to a repo and point out how the build system works (or breaks).

zubairkhanshinwari commented 3 years ago

import * as moment from 'moment'

can not work for me still not working...................