Skip to content

Commit

Permalink
Use yarn instead of npm (#706)
Browse files Browse the repository at this point in the history
Aside from somehow being orders of magnitude faster than npm, it also makes it easier to install into arbitrary directories, which is especially useful for potentially getting around a very strange docker bug on Johanna's system.

## Notes

* Upgraded apollo to get around apollographql/apollo-tooling#1329.

* I tried using a tool to convert our `package-lock.json` to a `yarn.lock` but it didn't work, so I just built our `yarn.lock` directly from the `package.json`.  This has resulted in some intermediate dependencies being different, which ultimately required me to upgrade a few packages.

    * One such package was `@testing-library/react`, which uncovered #707 (which we will deal with separately).
  • Loading branch information
toolness committed Jun 28, 2019
1 parent 9e51f9b commit 668710a
Show file tree
Hide file tree
Showing 16 changed files with 9,262 additions and 14,475 deletions.
16 changes: 8 additions & 8 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ jobs:
build:
working_directory: ~/tenants2
docker:
- image: justfixnyc/tenants2_base:0.4
- image: justfixnyc/tenants2_base:0.5
environment:
PIPENV_VENV_IN_PROJECT: true
DATABASE_URL: postgis://justfix@localhost/justfix
Expand All @@ -19,19 +19,19 @@ jobs:
steps:
- checkout
- restore_cache:
key: tenants2-take2-{{ .Branch }}-{{ checksum "Pipfile.lock" }}-{{ checksum "package-lock.json" }}-{{ checksum "requirements.production.txt" }}
key: tenants2-take2-{{ .Branch }}-{{ checksum "Pipfile.lock" }}-{{ checksum "yarn.lock" }}-{{ checksum "requirements.production.txt" }}
- run:
command: |
pipenv sync --dev
pipenv run pip install -r requirements.production.txt
# This will ensure that 'npm prepare' scripts on dependencies are run.
# For more details, see: https://stackoverflow.com/a/52767310
npm config set unsafe-perm true
yarn config set unsafe-perm true
npm install --no-save
yarn install --frozen-lockfile
- save_cache:
key: tenants2-take2-{{ .Branch }}-{{ checksum "Pipfile.lock" }}-{{ checksum "package-lock.json" }}-{{ checksum "requirements.production.txt" }}
key: tenants2-take2-{{ .Branch }}-{{ checksum "Pipfile.lock" }}-{{ checksum "yarn.lock" }}-{{ checksum "requirements.production.txt" }}
paths:
- ".venv"
- "node_modules"
Expand All @@ -49,9 +49,9 @@ jobs:
node querybuilder.js --version
node commondatabuilder.js --version
npm run build
npm test -- --runInBand
npm run lint
yarn build
yarn test -- --runInBand
yarn lint
pipenv run "pytest" --cov-report xml:./coverage/python/coverage.xml
- run:
name: CodeClimate combine and upload coverage
Expand Down
5 changes: 4 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ ENV NODE_VERSION=8

RUN curl -sL https://deb.nodesource.com/setup_${NODE_VERSION}.x | bash - \
&& curl -s https://packagecloud.io/install/repositories/github/git-lfs/script.deb.sh | bash - \
&& curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \
&& echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list \
&& apt-get update \
&& apt-get install -y \
nodejs \
yarn \
# Install the CLIs for databases so we can use 'manage.py dbshell'.
postgresql-client \
# Add support for GeoDjango.
Expand All @@ -23,4 +26,4 @@ RUN curl -sL https://deb.nodesource.com/setup_${NODE_VERSION}.x | bash - \
&& rm -rf /var/lib/apt/lists/* \
&& pip install pipenv

ENV PATH /tenants2/node_modules/.bin:$PATH
ENV PATH /tenants2/node_modules/.bin:/node_modules/.bin:$PATH
11 changes: 7 additions & 4 deletions Dockerfile.web
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ ENV NODE_VERSION=8

RUN curl -sL https://deb.nodesource.com/setup_${NODE_VERSION}.x | bash - \
&& curl -s https://packagecloud.io/install/repositories/github/git-lfs/script.deb.sh | bash - \
&& curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \
&& echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list \
&& apt-get update \
&& apt-get install -y \
nodejs \
yarn \
# Install the CLIs for databases so we can use 'manage.py dbshell'.
postgresql-client \
# Add support for GeoDjango.
Expand All @@ -30,7 +33,7 @@ RUN curl -sL https://deb.nodesource.com/setup_${NODE_VERSION}.x | bash - \
&& rm -rf /var/lib/apt/lists/* \
&& pip install pipenv

ENV PATH /tenants2/node_modules/.bin:$PATH
ENV PATH /tenants2/node_modules/.bin:/node_modules/.bin:$PATH

# This is where we start to deviate from the base Dockerfile.

Expand All @@ -55,14 +58,14 @@ RUN pipenv install --system --keep-outdated \

WORKDIR /tenants2

COPY package*.json /tenants2/
COPY package.json yarn.lock /tenants2/

# Make sure we run as a non-root user.
RUN useradd -m myuser
RUN chown myuser /tenants2
USER myuser

RUN npm install --no-save
RUN yarn install --frozen-lockfile

ADD --chown=myuser:myuser . /tenants2/

Expand All @@ -72,7 +75,7 @@ ARG IS_GIT_REPO_PRISTINE
ENV GIT_REVISION=$GIT_REVISION
ENV IS_GIT_REPO_PRISTINE=$IS_GIT_REPO_PRISTINE

RUN npm run build \
RUN yarn build \
#
# We specify 'USE_DEVELOPMENT_DEFAULTS=yup' and a fake
# database URL for this single command so that it can be run
Expand Down
14 changes: 7 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ This is an attempt at creating a new Tenants app for JustFix.
makes development much easier. But if you'd really rather set
everything up without Docker, read on!

You'll need Python 3.7.0 and [pipenv][], as well as Node 8 and
You'll need Python 3.7.0 and [pipenv][], as well as Node 8, yarn, and
[Git Large File Storage (LFS)][git-lfs]. You will also need to
set up Postgres version 10 or later.

Expand All @@ -32,8 +32,8 @@ Then set up the front-end and configure it to
continuously re-build itself as you change the source code:

```
npm install
npm start
yarn
yarn start
```

Then, in a separate terminal, you'll want to instantiate
Expand Down Expand Up @@ -100,10 +100,10 @@ pytest
To run the front-end Node/TypeScript tests, use:

```
npm test
yarn test
```

You can also use `npm run test:watch` to have Jest
You can also use `yarn test:watch` to have Jest
continuously watch the front-end tests for changes and
re-run them as needed.

Expand Down Expand Up @@ -187,7 +187,7 @@ Client-side GraphQL code is generated as follows:
the configuration in
[`frontend/lib/queries/autogen-config.toml`](frontend/lib/queries/autogen-config.toml).

3. The querybuilder, which runs as part of `npm start`, will notice
3. The querybuilder, which runs as part of `yarn start`, will notice
changes to any of these raw queries *or* `autogen-config.toml`
*or* the server's `schema.json`, and do the following:

Expand All @@ -206,7 +206,7 @@ Client-side GraphQL code is generated as follows:
to a file that is created next to the original `.graphql` file
(e.g., `SimpleQuery.ts`).

If the developer prefers not to rely on `npm start`
If the developer prefers not to rely on `yarn start`
to automatically rebuild queries for them, they can also manually
run `node querybuilder.js`.

Expand Down
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ services:
service: base_app
volumes:
- .:/tenants2:delegated
command: npm start
command: yarn start
db:
image: mdillon/postgis:10-alpine
volumes:
Expand All @@ -29,5 +29,6 @@ services:
- POSTGRES_USER=justfix
volumes:
node-modules:
unused-node-modules:
python-venv:
pgdata:
10 changes: 8 additions & 2 deletions docker-services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,21 @@ services:
# in these directories won't get mixed up with Darwin or
# other non-Linux native code compiled on the Docker Host.
- python-venv:/venv/
- node-modules:/tenants2/node_modules/
# This is really weird: some OS X systems behave erratically when
# we try overlaying and then populating our own node_modules atop the
# user's repository checkout, so instead we're going to mount an
# *empty* node_modules in that place and use /node_modules (in the
# root of the container) to store all our things.
- unused-node-modules:/tenants2/node_modules/
- node-modules:/node_modules/
environment:
# This is used by pipenv, but not really documented anywhere:
# https://github.com/pypa/pipenv/blob/master/pipenv/environments.py#L208
- VIRTUAL_ENV=/venv

- PYTHONUNBUFFERED=yup
- DDM_VENV_DIR=/venv
- DDM_USER_OWNED_DIRS=/venv:/tenants2/node_modules
- DDM_USER_OWNED_DIRS=/venv:/node_modules
- DDM_HOST_USER=justfix
- DDM_IS_RUNNING_IN_DOCKER=yup
- CHOKIDAR_USEPOLLING=1
Expand Down
2 changes: 1 addition & 1 deletion frontend/lib/pages/tests/login-page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ test('login page sets "next" input to expected value', () => {
url: '/login?next=/bop',
server: { originURL: 'https://blarg.com' }
});
pal.rr.getByText(/Sign in/i);
pal.rr.getAllByText(/Sign in/i);
expect(pal.getElement('input', '[name="next"]').value).toEqual('https://blarg.com/bop');
});

Expand Down
2 changes: 1 addition & 1 deletion frontend/lib/pages/tests/onboarding-step-1.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('onboarding step 1 page', () => {
}
}
});
const input = pal.rr.getByLabelText(/address/i) as HTMLInputElement;
const input = pal.rr.getAllByLabelText(/address/i)[0] as HTMLInputElement;
expect(input.value).toEqual('150 DOOMBRINGER STREET, Manhattan');
});

Expand Down
10 changes: 5 additions & 5 deletions frontend/lib/tests/rtl-pal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as rt from 'react-testing-library'
import * as rt from '@testing-library/react'
import { getElement } from '../util';

/**
Expand Down Expand Up @@ -70,9 +70,9 @@ export default class ReactTestingLibraryPal {
* given label text or a regular expression matching the label text.
*/
getFormField(label: string|RegExp): HTMLInputElement {
return this.rr.getByLabelText(label, {
return this.rr.getAllByLabelText(label, {
selector: 'input, select'
}) as HTMLInputElement;
})[0] as HTMLInputElement;
}

/** Send a keyDown event to the given form field with the give key code. */
Expand All @@ -90,9 +90,9 @@ export default class ReactTestingLibraryPal {

/** Retrieve a modal dialog with the given label in the render result. */
getDialogWithLabel(matcher: RegExp|string): HTMLDivElement {
return this.rr.getByLabelText(matcher, {
return this.rr.getAllByLabelText(matcher, {
selector: 'div[role="dialog"]'
}) as HTMLDivElement;
})[0] as HTMLDivElement;
}

/** Quick access to rt.cleanup(), which can be used in afterEach() calls. */
Expand Down
9 changes: 8 additions & 1 deletion frontend/querybuilder/querybuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,19 @@ import { deleteStaleTsFiles } from './stale-ts-files';
import { AutogenContext } from './autogen-graphql/context';
import { loadAutogenConfig } from './autogen-graphql/config';

/** Find the path to Apollo's command-line interface (CLI) script. */
function findApolloCliScript(): string {
// This should be something like '/foo/node_modules/apollo/lib'.
const apolloPath = path.dirname(require.resolve('apollo'));
return path.normalize(path.join(apolloPath, '..', 'bin', 'run'));
}

/**
* Run Apollo codegen:generate if needed, returning 0 on success, nonzero on errors.
*/
export function runApolloCodegen(): number {
const child = child_process.spawnSync('node', [
'node_modules/apollo/bin/run',
findApolloCliScript(),
'codegen:generate',
'--includes', QUERIES_GLOB,
'--localSchemaFile', GRAPHQL_SCHEMA_PATH,
Expand Down
2 changes: 1 addition & 1 deletion frontend/safe_mode/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const FILENAME = 'safe-mode.js';
function uglify() {
console.log(`Uglifying ${FILENAME}.`);
try {
execSync('npm run safe_mode_snippet', { stdio: 'inherit' });
execSync('yarn safe_mode_snippet', { stdio: 'inherit' });
} catch (e) {
console.log(chalk.redBright(`Uglification failed!`));
}
Expand Down
4 changes: 3 additions & 1 deletion frontend/webpack/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ function getCommonPlugins() {
DISABLE_WEBPACK_ANALYZER,
DISABLE_DEV_SOURCE_MAPS,
ENABLE_WEBPACK_CONTENT_HASH
})
}),
// https://github.com/webpack/webpack/issues/3078
new webpack.IgnorePlugin(/\/iconv-loader$/)
];

return plugins;
Expand Down

0 comments on commit 668710a

Please sign in to comment.