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

Importing moment in the TypeScript skeleton either breaks the app or the unit tests #606

Closed
bfil opened this issue Jul 14, 2016 · 40 comments

Comments

Projects
None yet
@bfil
Copy link
Contributor

commented Jul 14, 2016

I'm submitting a bug report

I'm using moment in my source code, and I usually import it as follows:

import * as moment from 'moment';

This works fine when opening the application in the browser, but when running the tests, the following error is thrown: moment is not a function

If I change the import to

import moment from 'moment';

then the tests pass, but when opening the application in the browser the following error is thrown: moment_1.default is not a function

Aurelia Skeleton Version
skeleton-typescript

aurelia-framework version:
1.0.0-rc.1.0.2

Operating System:
OSX 10.x

Node Version:
6.2.0

NPM Version:
3.8.9

JSPM OR Webpack AND Version
JSPM 0.16.32

Browser:
all

Language:
TypeScript 1.8.10 / 2.0.0

Current behavior:
Using import * as someModule from 'some-module', results in different transpiled version of the source code in the tests, which leads to errors being thrown inconsistently.

Expected/desired behavior:
It would be ideal if the code build by the gulp tasks would be used by the tests, or at least as similar as possible.

What is the motivation / use case for changing the behavior?
Unable to import global modules like moment without either breaking the app or the tests

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

Seems to me the tests in the JSPM are somehow set to incorrectly transpile the code, thus failing. I think @bfil is correct about why this currently breaks. Any ideas?

@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

@niieani Is the test configuration using a different module format perhaps?

@bfil

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2016

@EisenbergEffect as I pointed out in Gitter, the module specified in typescriptOptions might be overridden by SystemJS, take a look here:

https://github.com/systemjs/systemjs/blob/master/lib/legacy-transpiler.js#L80

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

@EisenbergEffect looks like that. In any case, the usage import x from 'x' when module x doesn't expose any default is a non-standard behavior and we should avoid it.

@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

Ok, so we need two fixes:

  • Make sure the docs have the correct syntax.
  • Figure out how to fix the test configuration to use the correct compilation options.

Is that correct?

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

Yes, exactly. I've started fixing some docs here: aurelia/binding#453 cause we've had some people complain that code copied from the docs doesn't work: aurelia/binding#452.

@bfil

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2016

Sorry @niieani but I'm not sure how changing the import helps, if in my date-format.ts I change the import to import moment from 'moment' then that throws an error in my Aurelia app:

TypeError: moment_1.default is not a function

So, how does this help? Here's how my date-format value converter gets transpiled to:

(function(System, SystemJS) {define(["require", "exports", 'moment'], function (require, exports, moment_1) {
    "use strict";
    var DateFormatValueConverter = (function () {
        function DateFormatValueConverter() {
        }
        DateFormatValueConverter.prototype.toView = function (value, format) {
            return moment_1.default(value).format(format);
        };
        return DateFormatValueConverter;
    }());
    exports.DateFormatValueConverter = DateFormatValueConverter;
});

})(System, System);

I've been fighting this for hours so I might be missing something here..

@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

The correct way to import it is:

import * as moment from 'moment';

But there's something going wrong with the TS configuration.

@bfil

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2016

Ok good, then it makes sense. I've been staring at this for too long.

I've branched my skeleton-navigation fork if you want to quickly reproduce the issue:

https://github.com/bfil/skeleton-navigation/tree/moment-tests-bug

A simple gulp test will do.

@guidorice

This comment has been minimized.

Copy link

commented Jul 15, 2016

@AshleyGrant

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

I tested this in the ESNext skeleton. I have to do import moment from 'moment'; for it to work.

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

@AshleyGrant does import * as moment from 'moment'; not work, or did you test both versions?

import moment from 'moment'; is non-standard for a package without a default export, so we should avoid it. If this is not working then we need to fix our Babel configuration.

@mradzinski

This comment has been minimized.

Copy link

commented Jul 15, 2016

Although import moment from 'moment'; throws an error on terminal saying that moment has no default export, it seems to work on the browser. On the other hand import * as moment from 'moment'; throws no error but fails to work on the browser ("moment is not a function"). I'm using the Typescript skeleton with Webpack.

@AshleyGrant

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

Yeah, when I run import * as moment from 'moment';, I have to do moment.default() to call the moment function.

@AshleyGrant

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

This is all while running in the browser, btw.

@mradzinski

This comment has been minimized.

Copy link

commented Jul 15, 2016

moment.default().format() works when using import * as moment from 'moment';, but it also prints an error on the terminal: Property 'default' does not exist on type 'typeof moment'.

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

Which webpack version are you guys using? There was a bug in Webpack which caused this, but was recently fixed. It works properly for me with the latest skeleton.

@mradzinski

This comment has been minimized.

Copy link

commented Jul 15, 2016

@niieani I'm using the latest version of the Skelton. Just to make sure I cloned a fresh version before posting

@guidorice

This comment has been minimized.

Copy link

commented Jul 15, 2016

@niieani I just did npm install which gets the webpack version from package.json in skeleton-navigation-1.0.0-beta.3.0.1/skeleton-typescript-webpack. ( "webpack": ">=2.1.0-beta.13 || ^2.1.0")

I just tried it with a fresh copy of the .zip of the skeleton, and first uninstalled my global webpack (1.13.1). I don't think it makes a difference what webpack is installed globally. I am not using the command line webpack with this skeleton. But it's uninstalled now.

Now npm start fails with this exception. Maybe there were some new things broken with the latest skeleton now?

.../node_modules/extract-text-webpack-plugin/index.js:108
throw new Error("Breaking change: ExtractTextPlugin now only takes a single argument. Either an options " +
^
Error: Breaking change: ExtractTextPlugin now only takes a single argument. Either an options object or the name of the result file.
Example: if your old code looked like this:
new ExtractTextPlugin('css/[name].css', { disable: false, allChunks: true })

You would change it to:
new ExtractTextPlugin({ filename: 'css/[name].css', disable: false, allChunks: true })

The available options are:
filename: string
allChunks: boolean
disable: boolean

@mradzinski

This comment has been minimized.

Copy link

commented Jul 15, 2016

For me npm start fails even worse, complaining it cannot find a module of webpack:

Error: Cannot find module 'webpack/bin/config-yargs'
    at Function.Module._resolveFilename (module.js:440:15)
    at Function.Module._load (module.js:388:25)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/***/node_modules/webpack-dev-server/bin/webpack-dev-server.js:24:1)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)

I already deleted node_modules folder and performed a clean npm install but it keeps failing.

@mradzinski

This comment has been minimized.

Copy link

commented Jul 15, 2016

@guidorice @niieani If I'm not mistake, some of the @easy-webpack configs (like the config-css) use the ExtractTextPlugin

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 16, 2016

@mradzinski @guidorice Seems like ExtractTextPlugin has been undergoing some heavy changes in the last 12 hours; I've updated the problematic package to the new syntax. Alternatively you can get around the issue by passing extractText: false to config-css. Note that all of this is off topic, not actually related to the problem at hand.

@bfil

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

FYI: for the time being I've added an horrible workaround to make moment work correctly both in the browser and in the tests:

import * as moment from 'moment';
if(moment.default) {
  moment = moment.default;
}
@AshleyGrant

This comment has been minimized.

Copy link
Member

commented Jul 18, 2016

Simpler workaround: no more unit tests! 👅

@jsobell

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2016

import * as moment from 'moment';

export class DateFormatValueConverter {
  toView(value) {
    return (moment as any).default(value).format('YYYY-MM-DD HH:mm');
  }
  fromView(value) {
    return (moment as any).default(value,'YYYY-MM-DD HH:mm');
  }
}
@ralexand56

This comment has been minimized.

Copy link

commented Jul 27, 2016

Any updates on this? So frustrating that one of the most popular javascript libraries has all these issues running on the skeletons.

@niieani

This comment has been minimized.

Copy link
Member

commented Jul 28, 2016

We're looking into migrating to JSPM 0.17, this might be better to fix in that version.

@bfil

This comment has been minimized.

Copy link
Contributor Author

commented Dec 13, 2016

@EisenbergEffect has this been fixed? if so, any details on what needs to be changed to make this play nicely? Thanks

@niieani niieani added the JSPM / Gulp label Dec 17, 2016

@mariettamx

This comment has been minimized.

Copy link

commented Mar 24, 2017

For me it worked to install moment-es6 module , declare both (moment and moment-es6) in aurelia.json file, and to link the typings of the library in typings/index.d.ts:

reference path="../node_modules/moment/moment.d.ts"
reference path="../node_modules/moment-es6/index.d.ts"

@Keethanjan

This comment has been minimized.

Copy link

commented Apr 19, 2017

@EisenbergEffect I am still having troubles to get this working accordingly.
Could one of you guys take a look at it again? Thanks.

@Krossmaskinen

This comment has been minimized.

Copy link

commented Aug 4, 2017

I'm using the latest version of typescript skeleton. Still having this problem.

@peterpeterparker

This comment has been minimized.

Copy link

commented Aug 26, 2017

While doing

import moment from 'moment';

I was facing the error

'"/Users/me/Documents/project/node_modules/moment/moment"' has no default export.

So I just added the following to the compilerOptions in tsconfig.json`

"allowSyntheticDefaultImports": true

which solved my issue. If that could help someone...

@jordilondoner

This comment has been minimized.

Copy link

commented Sep 27, 2017

I run into the same situation where I had two files:

  1. sourceCode.ts where I used import * as moment from 'moment';
  2. sourceCode.test.ts where I used import * as moment from 'moment';

with the test throwing the error: moment is not a function.

When I tried to change both to import moment from 'moment'; then the test run ok but the actual resulting code crashed in the browser.

The solution was:
{ "jest": { "globals": { "ts-jest": { "skipBabel": true } } } }
I tried it after reading ts-jest docs but being completely honest it doesn t makes a lot of sense as I do have the setting for allowSyntheticDefaultImports in my ts-config...

@gerbendekker

This comment has been minimized.

Copy link

commented Oct 26, 2017

Using the Microsoft.AspNetCore.SpaTemplates here, with webpack 2.3.3.
I came by this issue and this worked for me to get the ES6 module:
import moment from 'moment/src/moment';

@vance

This comment has been minimized.

Copy link

commented Mar 8, 2018

also broken when using import * as moment from 'moment-timezone' just sayin'

@afraisse

This comment has been minimized.

Copy link

commented Apr 24, 2018

Still having the exact same problem, why is this closed ?

@AshleyGrant

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

Are you having the problem with JSPM or with Webpack? I can confirm that import * as moment from 'moment'; works just fine in an Aurelia CLI based application using Webpack 4. Strangely, import moment from 'moment'; worked with a Webpack 3 based app built with the Aurelia CLI, but I had to switch to the * as syntax with Webpack 4.

@ryanrombough

This comment has been minimized.

Copy link

commented Jun 13, 2018

I am not using Aurelia skeleton-navigation, but I used this thread to help troubleshoot when I had the same problem. None of the solutions here worked for me. I fixed the issue by upgrading to typescript >v2.7 and adding "esModuleInterop": true, to the compilerOptions in my tsconfig.json. This allowed me to use import moment from 'moment' in my typescript files. Hope that helps anyone who stumbles on this in the future!

Here's a link to the relevant typescript documentation: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-7.html#support-for-import-d-from-cjs-form-commonjs-modules-with---esmoduleinterop

hatton added a commit to SayMoreX/saymore-x that referenced this issue Aug 17, 2018

simplify date handling
Now we just try to quickly get the date into a yyyy-mm-dd format, and don't mess with it after that.

Had to change tsconfig settings and how commonjs modules are imported in order to get both jest and the app to both be happy with moment. See aurelia/skeleton-navigation#606
@silbinarywolf

This comment has been minimized.

Copy link

commented Sep 20, 2018

I'm having this exact problem, but it's the opposite import syntax and for the flatpickr library.
If I have import flatpickr from 'flatpickr'; then everything compiles correctly for the browser.
However then I get the following error thrown from Jest:

 TypeError: flatpickr_1.default is not a function

      43 |              }
      44 |              // Setup flatpicker
    > 45 |              this.flatpickr = flatpickr(this.inputElem, {
         |                               ^
      46 |                      enableTime: false,
      47 |                      altInput: true,

If I swap this import to:

import * as flatpickr from 'flatpickr';

Then my Jest tests pass but now flatpickr wont run in the browser.

Finally, if I try to override my jest TypeScript config file, this causes "ts-jest" to no longer force the "module" config option to "commonjs".

However that just causes the component.viewModel to stop working suddenly.

"modulePaths": [
  "<rootDir>/src",
  "<rootDir>/node_modules"
],
"moduleFileExtensions": [
  "js",
  "jsx",
  "json",
  "ts",
  "tsx"
],
"cacheDirectory": ".jest/cache",
"transform": {
  "^.+\\.tsx?$": "ts-jest",
  "^.+\\.jsx?$": "babel-jest"
},
"transformIgnorePatterns": [
  "<rootDir>/node_modules/(?!lodash-es)"
],
"testRegex": "\\.spec\\.(ts|js)x?$",
"setupFiles": [
  "<rootDir>/test/jest-pretest.ts"
],
"testEnvironment": "node",
"moduleNameMapper": {
  "~(.*)$": "<rootDir>/src/$1"
},
"globals": {
  "ts-jest": {
    "useBabelrc": true,
    "tsConfigFile": "tsconfig.jest.json"
  }
}

tsconfig.jest.json:

{
	"extends": "./tsconfig",
	"compilerOptions": {}
}

Output:

 ● Datepicker › test arrow down opening flatpickr calendar

    TypeError: Cannot read property 'flatpickr' of undefined

    > 77 |                      inputElem = component.viewModel.flatpickr._input;

I'm going to see if updating ts-jest resolves these problems.

@silbinarywolf

This comment has been minimized.

Copy link

commented Sep 20, 2018

I was able to get flatpickr and moment working.
I needed to update to the latest ts-jest version from 23.1.3 to 23.10.0

Needed to change my imports to both look like this across my project:

import flatpickr from 'flatpickr';
import moment from 'moment';

tsconfig.json

{
	"compilerOptions": {
		"baseUrl": ".",
		"emitDecoratorMetadata": true,
		"experimentalDecorators": true,
		"lib": [
			"es6",
			"dom"
		],
		"module": "esnext",
		"moduleResolution": "node",
                //
                // NOTE: allowSyntheticDefaultImports was required to fix import issues
                //
		"allowSyntheticDefaultImports": true,
		"noUnusedParameters": false,
		"noUnusedLocals": false,
		"paths": {
			"~/*": ["src/*"],
			"~": ["src"]
		},
		"skipLibCheck": true,
		"target": "es5",
		"typeRoots": [
			"node",
			"node_modules/@types",
			"custom_typings"
		]
	},
	"compileOnSave": true,
	"rootDirs": [
		"src"
	],
	"include": [
		"src/**/*",
		"test/**/*",
		"custom_typings/**/*"
	],
	"exclude": [
		"dist",
		"node_modules",
		"static"
	]
}

jest-pretest.js

import 'aurelia-polyfills';
import { Options } from 'aurelia-loader-nodejs';
import { globalize } from 'aurelia-pal-nodejs';
import * as path from 'path';
Options.relativeToDir = path.join(__dirname, 'unit');

globalize();

// Fix HTMLCollection not working correctly when importing flatpickr + running jest.
global['HTMLCollection'] = window['HTMLCollection'];

// Fix using "new KeyboardEvent" in jest tests.
global['KeyboardEvent'] = window['KeyboardEvent'];

// Add polyfill for "window.requestAnimationFrame" not working correctly running jest.
if (!window['requestAnimationFrame']) {
	window['requestAnimationFrame'] = function(callback) {
		return window.setTimeout(callback, 0);
	};
}

Jest config:

"modulePaths": [
  "<rootDir>/src",
  "<rootDir>/node_modules"
],
"moduleFileExtensions": [
  "js",
  "jsx",
  "json",
  "ts",
  "tsx"
],
"cacheDirectory": ".jest/cache",
"transform": {
  "^.+\\.jsx?$": "babel-jest",
  "^.+\\.tsx?$": "ts-jest"
},
"transformIgnorePatterns": [
  "<rootDir>/node_modules/(?!lodash-es)"
],
"testRegex": "\\.spec\\.(ts|js)x?$",
"setupFiles": [
  "<rootDir>/test/jest-pretest.ts"
],
"testEnvironment": "node",
"moduleNameMapper": {
  "~(.*)$": "<rootDir>/src/$1"
},
"globals": {
  "ts-jest": {
    "babelConfig": true
  }
}

.babelrc

{
	"env": {
		"test": {
			"plugins": [
				// This is here so that "lodash-es" will transpile when running jest.
				// Related issue: https://github.com/kulshekhar/ts-jest/issues/494
				"transform-es2015-modules-commonjs",
			]
		}
	}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.