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

Switch to pnpm #1404

Merged
merged 1 commit into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 12 additions & 2 deletions .github/workflows/ci-frontend-size-limit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,18 @@ jobs:
env:
CI_JOB_NUMBER: 1
steps:
- uses: actions/checkout@v1
- uses: andresz1/size-limit-action@v1
- name: Checkout
uses: actions/checkout@v1

- name: Install pnpm
uses: pnpm/action-setup@v2.0.1
id: pnpm-install
with:
version: 7
run_install: false

- name: Check bundle size
uses: andresz1/size-limit-action@v1
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
directory: frontend
142 changes: 114 additions & 28 deletions .github/workflows/ci-frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,40 @@ jobs:
node: [16.15.1]

steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
- name: Checkout
uses: actions/checkout@v2

- name: Install node
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node }}

- run: npm ci --loglevel warn
working-directory: ./frontend
- name: Install pnpm
uses: pnpm/action-setup@v2.0.1
id: pnpm-install
with:
version: 7
run_install: false

- name: Get pnpm store directory
id: pnpm-cache
run: |
echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"

- uses: actions/cache@v2
- name: Setup pnpm cache
uses: actions/cache@v3
with:
path: ${{ github.workspace }}/frontend/node_modules/.cache
key: ${{ runner.os }}-${{ hashFiles('**/package-lock.json') }}
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-node-
${{ runner.os }}-pnpm-store-

- run: npm run check:translation
- name: Install dependencies
run: pnpm i --loglevel warn
working-directory: ./frontend

- name: Run translations check
run: pnpm run check:translation
working-directory: ./frontend

check-typescript:
Expand All @@ -47,22 +65,40 @@ jobs:
node: [16.15.1]

steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
- name: Checkout
uses: actions/checkout@v2

- name: Install node
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node }}

- run: npm ci --loglevel warn
working-directory: ./frontend
- name: Install pnpm
uses: pnpm/action-setup@v2.0.1
id: pnpm-install
with:
version: 7
run_install: false

- name: Get pnpm store directory
id: pnpm-cache
run: |
echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"

- uses: actions/cache@v2
- name: Setup pnpm cache
uses: actions/cache@v3
with:
path: ${{ github.workspace }}/frontend/node_modules/.cache
key: ${{ runner.os }}-${{ hashFiles('**/package-lock.json') }}
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-node-
${{ runner.os }}-pnpm-store-

- run: npm run check:types
- name: Install dependencies
run: pnpm i --loglevel warn
working-directory: ./frontend

- name: Run type check
run: pnpm run check:types
working-directory: ./frontend

lint:
Expand All @@ -72,15 +108,40 @@ jobs:
node: [16.15.1]

steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
- name: Checkout
uses: actions/checkout@v2

- name: Install node
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node }}

- run: npm ci --loglevel warn
- name: Install pnpm
uses: pnpm/action-setup@v2.0.1
id: pnpm-install
with:
version: 7
run_install: false

- name: Get pnpm store directory
id: pnpm-cache
run: |
echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"

- name: Setup pnpm cache
uses: actions/cache@v3
with:
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-pnpm-store-

- name: Install dependencies
run: pnpm i --loglevel warn
working-directory: ./frontend

- run: npm run lint
- name: Run linters
run: pnpm run lint
working-directory: ./frontend

test:
Expand All @@ -90,18 +151,43 @@ jobs:
node: [16.15.1]

steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
- name: Checkout
uses: actions/checkout@v2

- name: Install node
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node }}

- run: npm ci --loglevel warn
- name: Install pnpm
uses: pnpm/action-setup@v2.0.1
id: pnpm-install
with:
version: 7
run_install: false

- name: Get pnpm store directory
id: pnpm-cache
run: |
echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"

- name: Setup pnpm cache
uses: actions/cache@v3
with:
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-pnpm-store-

- name: Install dependencies
run: pnpm i --loglevel warn
working-directory: ./frontend

- run: npm run test:coverage
- name: Collect tests coverage
run: pnpm run test --coverage
working-directory: ./frontend

- name: submit coverage
run: node ${{ github.workspace }}/frontend/node_modules/.bin/codecov
- name: Submit coverage
run: ${{ github.workspace }}/frontend/node_modules/.bin/codecov
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ RUN if [ -z "$SKIP_FRONTEND_BUILD" ] ; then \
ADD frontend/package.json /srv/frontend/package.json
ADD frontend/package-lock.json /srv/frontend/package-lock.json
WORKDIR /srv/frontend
RUN mkdir node_modules
RUN if [ -z "$SKIP_FRONTEND_BUILD" ] ; then \
CI=true npm ci --loglevel warn \
npm i -g pnpm && \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can use https://nodejs.org/api/corepack.html for activate pnpm
corepack pnpm install

Copy link
Collaborator Author

@akellbl4 akellbl4 Jul 11, 2022

Choose a reason for hiding this comment

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

Hm, I think its too early for corepack. It was introduced in v18 and it's experimental.
But worth considering when we move towards update to node18

UPD: Oh, I'm wrong it's available in v16.

CI=true pnpm i --loglevel warn \
else \
echo "skip frontend build" \
; fi
Expand Down
8 changes: 6 additions & 2 deletions Dockerfile.artifacts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@ FROM node:16.15.1-alpine as build-frontend-deps
ARG CI

ENV SKIP_FRONTEND_TEST=true
ENV CI=true

RUN apk add --no-cache --update git
ADD frontend/package.json /srv/frontend/package.json
ADD frontend/package-lock.json /srv/frontend/package-lock.json
RUN cd /srv/frontend && CI=true npm ci
RUN
cd /srv/frontend && \
apk add --no-cache --update git && \
npm i -g pnpm && \
pnpm i

FROM node:16.15.1-alpine as build-frontend

Expand Down
4 changes: 2 additions & 2 deletions frontend/app/components/auth/auth.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('<Auth/>', () => {

it('should show validation error for token', async () => {
StaticStore.config.auth_providers = ['email'];
jest.spyOn(api, 'emailSignin').mockImplementationOnce(async () => null);
const emailSignin = jest.spyOn(api, 'emailSignin').mockImplementationOnce(async () => null);

const { getByText, getByTitle, getByPlaceholderText } = render(<Auth />);

Expand All @@ -183,7 +183,7 @@ describe('<Auth/>', () => {
target: { value: 'email@email.com' },
});
fireEvent.click(getByText('Submit'));
await waitFor(() => expect(api.emailSignin).toBeCalled());
await waitFor(() => expect(emailSignin).toBeCalled());

expect(getByText('Back')).toHaveClass('auth-back-button');
expect(getByTitle('Close sign-in dropdown')).toHaveClass('auth-close-button');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function setup(overrideProps: Partial<Props> = {}, overrideConfig: Partial<typeo
id: '1',
...overrideProps,
} as Props;

// @ts-ignore
const CommentFormWithIntl = () => <CommentForm {...props} intl={useIntl()} />;

return render(<CommentFormWithIntl />);
Expand Down
3 changes: 1 addition & 2 deletions frontend/app/components/profile/profile.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import '@testing-library/jest-dom';
import { waitFor } from '@testing-library/preact';
import { fireEvent } from '@testing-library/dom';
import { waitFor, fireEvent } from '@testing-library/preact';

import { render } from 'tests/utils';
import * as api from 'common/api';
Expand Down
40 changes: 20 additions & 20 deletions frontend/app/components/select/select.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import '@testing-library/jest-dom';
import { fireEvent } from '@testing-library/preact';
import { fireEvent, screen, waitFor } from '@testing-library/preact';

import { render } from 'tests/utils';
import { Select } from './select';
Expand All @@ -14,32 +14,32 @@ const items = [

describe('<Select/>', () => {
it('should has static class names', () => {
const { container } = render(<Select items={items} selected={items[0]} />);
const selectElement = container.querySelector('.select-element');

expect(container.querySelector('.select')).toBeInTheDocument();
expect(container.querySelector('.select-arrow')).toBeInTheDocument();
expect(selectElement).toBeInTheDocument();
fireEvent.focus(selectElement as HTMLSelectElement);
expect(container.querySelector('.select_focused')).toBeInTheDocument();
render(<Select items={items} selected={items[0]} />);
expect(screen.getByRole('combobox')).toHaveClass('select-element');
expect(screen.getByTestId('select-root')).toHaveClass('select');
expect(screen.getByTestId('select-arrow')).toHaveClass('select-arrow');
});

it('should render selected item', () => {
const { container, getAllByText } = render(<Select items={items} selected={items[0]} />);
const selectedOption = container.querySelector('option');
render(<Select items={items} selected={items[0]} />);

expect(getAllByText(items[0].label)).toHaveLength(2);
const selectedItem = items[0];
const selectedOption = screen.getAllByRole<HTMLOptionElement>('option')[0];

expect(screen.getAllByText(selectedItem.label)).toHaveLength(2);
expect(selectedOption).toBeInTheDocument();
expect(selectedOption?.selected).toBeTruthy();
expect(selectedOption?.textContent).toBe(items[0].label);
expect(selectedOption.selected).toBeTruthy();
expect(selectedOption.textContent).toBe(selectedItem.label);
});

it('should highlight select on focus', async () => {
const { container } = render(<Select items={items} selected={items[0]} />);
const select = container.querySelector('select');

expect(container.querySelector('select')).toBeInTheDocument();
fireEvent.focus(select as HTMLSelectElement);
expect(container.querySelector('.rootFocused')).toBeInTheDocument();
render(<Select items={items} selected={items[0]} />);

fireEvent.focus(screen.getByRole('combobox'));
await waitFor(() => {
const rootElement = screen.getByTestId('select-root');
expect(rootElement).toHaveClass('select_focused');
expect(rootElement).toHaveClass('rootFocused');
});
});
});
3 changes: 2 additions & 1 deletion frontend/app/components/select/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ export function Select({ items, selected, size = 'md', ...props }: Props) {

return (
<span
data-testid="select-root"
className={clsx('select', styles.root, size && styles[size], {
[styles.rootFocused]: focus,
select_focused: focus,
[`select_${size}`]: size,
})}
>
{selectedItem.label}
<ArrowIcon size={iconSize[size]} className={clsx('select-arrow', styles.arrow)} />
<ArrowIcon data-testid="select-arrow" size={iconSize[size]} className={clsx('select-arrow', styles.arrow)} />
<select
{...props}
onFocus={() => setFocus(true)}
Expand Down
4 changes: 1 addition & 3 deletions frontend/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@ const config: Config = {
},
],
},
transformIgnorePatterns: ['node_modules/(?!(@testing-library/preact|preact|@github|lodash-es))'],
transformIgnorePatterns: ['node_modules/.pnpm/(?!(@testing-library|preact|@github|lodash-es))'],
moduleDirectories: ['node_modules', 'app'],
moduleNameMapper: {
'\\.css': 'identity-obj-proxy',
'\\.svg': '<rootDir>/app/__stubs__/svg.tsx',
'^react$': 'preact/compat',
'^react-dom$': 'preact/compat',
},
setupFiles: ['<rootDir>/jest.setup.ts'],
setupFilesAfterEnv: [
Expand Down
Loading