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

Upgrade TypeScript & other deps #588

Merged
merged 6 commits into from
May 13, 2020
Merged

Upgrade TypeScript & other deps #588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fc9eb97
Webapp dependency updates:
LachlanStuart May 13, 2020
fbbf76e
Graphql dependency update:
LachlanStuart May 13, 2020
4187ff8
Graphql dependency updates to resolve security alerts:
LachlanStuart May 13, 2020
ac7520a
Remove nodemon from graphql as it's no longer used
LachlanStuart May 13, 2020
4c62bfe
Raise test timeout
LachlanStuart May 13, 2020
644a875
Pull graphql coverage circleci fix from feat/scientific-publishing-am…
LachlanStuart May 13, 2020
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
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
- run:
name: Upload coverage
command: |
yarn run test --coverage
yarn run test --coverage --maxWorkers=1
npx codecov -p ../.. -F graphql


Expand Down
18 changes: 9 additions & 9 deletions metaspace/graphql/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"Vitaly Kovalev <vitaly.kovalev@embl.de>"
],
"scripts": {
"dev": "nodemon -e js,json,ts --require ts-node/register server.js",
"dev": "ts-node-dev server.js --respawn server.js",
"start": "ts-node server.js",
"test": "NODE_ENV=test jest",
"test-integration": "mocha --timeout 3000 tests/*RegrTest.js # Has not been maintained. Mocha removed due to security advisory in one of its dependencies",
Expand All @@ -25,7 +25,7 @@
},
"license": "ISC",
"dependencies": {
"@sentry/node": "5.4.0",
"@sentry/node": "^5.15.5",
"@types/amqplib": "^0.5.9",
"@types/aws-sdk": "^2.7.0",
"@types/bcrypt": "^2.0.0",
Expand Down Expand Up @@ -72,7 +72,7 @@
"json-schema-deref-sync": "^0.10.1",
"jsondiffpatch": "^0.3.9",
"jsonwebtoken": "^8.5.1",
"knex": "^0.19.5",
"knex": "0.20.15",
"lodash": "^4.17.4",
"merge-graphql-schemas": "^1.5.3",
"mkdirp": "^0.5.1",
Expand All @@ -91,7 +91,7 @@
"subscriptions-transport-ws": "^0.9.14",
"ts-node": "^8.3.0",
"typeorm": "0.2.18",
"typescript": "^3.6.2",
"typescript": "^3.9.2",
"uuid": "^3.3.2",
"winston": "^2.4.4"
},
Expand All @@ -101,12 +101,11 @@
"@types/node": "^8",
"@types/supertest": "^2.0.7",
"graphql-binding": "^2.2.2",
"jest": "^24.9.0",
"nodemon": "^1.19.1",
"jest": "25.5.4",
"random-js": "^2.0.0-rc2",
"request-promise": "^4.2.1",
"supertest": "^4.0.2",
"ts-jest": "^23.1.3",
"ts-jest": "25.5.0",
"ts-node-dev": "^1.0.0-pre.42"
},
"jest": {
Expand All @@ -127,15 +126,16 @@
"node"
],
"testEnvironment": "node",
"testTimeout": 15000,
"setupFilesAfterEnv": [
"<rootDir>/src/tests/setupTestFramework.ts"
],
"coveragePathIgnorePatterns": [
"node_modules",
"src/migrations"
],
"globalSetup": "<rootDir>/src/tests/globalSetup.js",
"globalTeardown": "<rootDir>/src/tests/globalTeardown.js",
"globalSetup": "<rootDir>/src/tests/globalSetup.ts",
"globalTeardown": "<rootDir>/src/tests/globalTeardown.ts",
"clearMocks": true
}
}
4 changes: 2 additions & 2 deletions metaspace/graphql/src/modules/project/controller/Mutation.membership.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {BackgroundData, createBackgroundData, validateBackgroundData} from '../.
import {DatasetProject as DatasetProjectModel} from '../../dataset/model';
import {In} from 'typeorm';
import {Context} from '../../../context';
import * as auth from '../../auth';
import * as authEmails from '../../auth/email';
import * as projectEmails from '../../groupOrProject/email';
import {createUserCredentials, verifyEmail} from '../../auth/operation';

Expand Down Expand Up @@ -148,7 +148,7 @@ describe('modules/project/controller (membership-related mutations)', () => {
});

test('Manager invites non-existent user, user creates account and accepts', async () => {
const invitationEmailSpy = jest.spyOn(auth, 'sendInvitationEmail');
const invitationEmailSpy = jest.spyOn(authEmails, 'sendInvitationEmail');
const email = 'newuser123@example.com';

await doQuery(inviteUserToProjectQuery, {projectId, email}, {context: managerContext});
Expand Down
14 changes: 8 additions & 6 deletions metaspace/graphql/src/tests/globalSetup.js → metaspace/graphql/src/tests/globalSetup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
require('ts-node/register');
const Knex = require('knex');
const config = require('../utils/config').default;
const {createConnection} = require('../utils/db');
const {DbSchemaName} = require('../utils/typeOrmConfig');
import * as Knex from 'knex';
import config from '../utils/config';
import {createConnection} from '../utils/db';
import {DbSchemaName} from '../utils/typeOrmConfig';

module.exports = async () => {
export = async () => {
Copy link
Member

Choose a reason for hiding this comment

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

this is new syntax for me, quickly looked up and found some old discussion here: microsoft/TypeScript#7185

is this for commonJS compat?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. module.exports = and export = are functionally equivalent. Both would be imported with globalSetup = require(...) / import * as globalSetup from '...'. In fact, TypeScript allows both forms of export, I just figured it would be better to match syntax with the rest of the file.

I thought it was an ES Modules thing, not a TypeScript thing, but I could be mistaken...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MDN and Node's docs don't have any mention of it, so I guess it is a TypeScript thing after all.


if (config.db.database === 'sm'
|| config.util.getConfigSources().some(({name}) => name.endsWith('development.js') || name.endsWith('production.js'))) {
Expand Down Expand Up @@ -43,6 +42,9 @@ module.exports = async () => {
GRANT ALL ON ALL TABLES IN SCHEMA public TO ${config.db.user}`);
await knex.destroy();

// For some reason the ts-jest transformer doesn't work automatically for the TypeORM migrations,
// so manually register ts-node before running migrations.
require('ts-node/register');
// Create a TypeORM connection just to apply migrations, so that parallel tests don't conflict during initialization
const conn = await createConnection();
await conn.runMigrations();
Expand Down
6 changes: 3 additions & 3 deletions ...space/graphql/src/tests/globalTeardown.js → ...space/graphql/src/tests/globalTeardown.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const Knex = require('knex');
const config = require('../utils/config').default;
import * as Knex from 'knex';
import config from '../utils/config';

module.exports = async () => {
export = async () => {
const knexAdmin = Knex({
client: 'postgres',
connection: {
Expand Down
Loading