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.

Sign up for GitHub

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

Jump to bottom

Add a TypeScript test and fix compiler errors #1903

Merged
merged 5 commits into from
Feb 1, 2018
Merged

Add a TypeScript test and fix compiler errors #1903

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
13304f3
add a typescript test for dist/typings
unstubbable Jan 23, 2018
b7335f1
fix import of EventEmitter (events has no default export)
unstubbable Jan 23, 2018
4965439
remove unused dependency buffer-xor
unstubbable Jan 23, 2018
65a713d
adjust npm scripts
unstubbable Jan 28, 2018
70fee6c
add cash-mv/cash-rm for windows support
unstubbable Jan 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
"test:only": "mocha",
"test:leak": "node --expose-gc test/leak/index.js",
"test:quick": "rollup -c && mocha -b",
"test:typescript": "mv src src_tmp; tsc -p test/typescript; mv src_tmp src",
"pretest-coverage": "npm run build",
"test-coverage": "rm -rf coverage/* && istanbul cover --report json node_modules/.bin/_mocha -- -u exports -R spec test/test.js",
"posttest-coverage": "remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.json -b dist && remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.lcov -t lcovonly -b dist && remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped -t html -b dist",
"ci": "npm run test-coverage && codecov < coverage/coverage-remapped.lcov",
"build": "git rev-parse HEAD > .commithash && rollup -c && tsc -p tsconfig-types.json && chmod a+x bin/rollup",
"watch": "rollup -cw",
"prepublishOnly": "npm run lint && npm run test:only && npm run test:leak",
"prepublishOnly": "npm run lint && npm run test:only && npm run test:leak && npm run test:typescript",
Copy link
Member

@lukastaegert lukastaegert Jan 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering your question when this test should run: As it appears that, at least on my system, this test is rather fast, I think it would be ok to include it in the test script. Otherwise the person publishing it would be the only one fixing issues here (i.e. probably me).
However I would add it after the regular test, i.e. npm run test:only && npm run test:typescript. Adding it to test will also make it run on CI. The ci script, on the other hand, seems to be unused at the moment.

Another issue I stumble upon is that the typings in the dist folder are never cleaned up which resulted in a lot of weird errors for me when running this test. My suggestion is to remove the dist/typings folder (or maybe even the dist folder) as part of the build script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 65a713d. I'm not a windows guy so I'm not sure whether commands like rm or mv are still a cross-platform problem these days. Since rm -rf is already used in the test-coveragescript I used the same in prebuild. If that's a concern we could use rimraf instead. I'm not sure about the mv though. Maybe cash-mv is an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, appveyor just answered my question. Will fix...

"prepare": "npm run build",
"lint": "eslint src browser bin test/test.js test/*/index.js test/utils test/**/_config.js"
},
Expand Down Expand Up @@ -53,7 +54,6 @@
"acorn-dynamic-import": "^3.0.0",
"ansi-escapes": "^3.0.0",
"buble": "^0.18.0",
"buffer-xor": "^2.0.2",
"chalk": "^2.3.0",
"codecov.io": "^0.1.6",
"console-group": "^0.3.1",
Expand Down
1 change: 0 additions & 1 deletion src/Graph.d.ts
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/Graph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/// <reference path="./Graph.d.ts" />
import * as acorn from 'acorn';
import injectDynamicImportPlugin from 'acorn-dynamic-import/lib/inject';
import { timeEnd, timeStart } from './utils/flushTime';
Expand Down
2 changes: 1 addition & 1 deletion src/watch/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import path from 'path';
import EventEmitter from 'events';
import { EventEmitter } from 'events';
import createFilter from 'rollup-pluginutils/src/createFilter.js';
import rollup, { InputOptions, OutputOptions, OutputChunk } from '../rollup/index';
import ensureArray from '../utils/ensureArray';
Expand Down
1 change: 1 addition & 0 deletions test/typescript/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import * as rollup from '../../dist/typings/node-entry';
16 changes: 16 additions & 0 deletions test/typescript/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"compilerOptions": {
"noEmit": true,
"strict": true,
"pretty": true,
"lib": [
"es2015"
]
},
"include": [
"../../dist"
],
"files": [
"./index.ts"
]
}