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

Enable support for React 18 #432

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
7,626 changes: 5,751 additions & 1,875 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
"i18n_extract": "fedx-scripts formatjs extract packages/**/**/*.{js,jsx,ts,tsx}"
},
"devDependencies": {
"@commitlint/config-conventional": "17.8.1",
"conventional-changelog-conventionalcommits": "^5.0.0",
"husky": "6.0.0",
"lerna": "6.6.2"
"@commitlint/config-conventional": "17.8.1",
"conventional-changelog-conventionalcommits": "^5.0.0",
"husky": "6.0.0",
"lerna": "6.6.2"
}
}
36 changes: 18 additions & 18 deletions packages/catalog-search/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@edx/frontend-enterprise-catalog-search",
"version": "10.8.0",
"name": "@adamstankiewicz/openedx-frontend-enterprise-catalog-search",
"version": "10.9.3",
"description": "Components related to Enterprise catalog search.",
"repository": {
"type": "git",
Expand All @@ -11,16 +11,16 @@
"patterns": [
"src"
],
"extensions": "js,jsx"
"extensions": "js,jsx,ts,tsx"
}
},
"scripts": {
"dev": "npx npm-watch build",
"clean": "make clean",
"build": "make build",
"i18n_extract": "BABEL_ENV=i18n fedx-scripts babel src --quiet > /dev/null",
"lint": "fedx-scripts eslint --ext .js --ext .jsx .",
"lint:fix": "fedx-scripts eslint --fix --ext .js --ext .jsx .",
"lint": "fedx-scripts eslint --ext .js --ext .jsx --ext .ts --ext .tsx .",
"lint:fix": "fedx-scripts eslint --fix --ext .js --ext .jsx --ext .ts --ext .tsx .",
"snapshot": "fedx-scripts jest --updateSnapshot",
"test": "fedx-scripts jest --coverage --passWithNoTests",
"test:watch": "npm run test -- --watch"
Expand All @@ -37,31 +37,31 @@
},
"sideEffects": false,
"dependencies": {
"@edx/frontend-enterprise-utils": "^9.1.0",
"@edx/frontend-enterprise-utils": "npm:@adamstankiewicz/openedx-frontend-enterprise-utils@^9.2.1",
"classnames": "2.5.1",
"lodash.debounce": "4.0.8",
"prop-types": "15.8.1"
},
"devDependencies": {
"@edx/browserslist-config": "1.5.0",
"@edx/frontend-platform": "8.1.5",
"@openedx/frontend-build": "14.2.2",
"@openedx/paragon": "21.13.1",
"@edx/frontend-platform": "npm:@adamstankiewicz/openedx-frontend-platform@^8.2.1",
"@openedx/frontend-build": "npm:@adamstankiewicz/openedx-frontend-build@^14.3.0",
"@openedx/paragon": "22.15.1",
"@testing-library/dom": "10.4.0",
"@testing-library/jest-dom": "5.17.0",
"@testing-library/react": "12.1.5",
"@testing-library/react-hooks": "3.7.0",
"@testing-library/user-event": "13.5.0",
"react": "17.0.2",
"react-dom": "17.0.2",
"@testing-library/react": "16.2.0",
"@testing-library/user-event": "14.6.1",
"react": "18.3.1",
"react-dom": "18.3.1",
"react-instantsearch-dom": "6.40.4",
"react-router-dom": "6.29.0",
"react-test-renderer": "17.0.2"
"react-test-renderer": "18.3.1"
},
"peerDependencies": {
"@edx/frontend-platform": "^7.0.0 || ^8.0.0",
"@openedx/paragon": "^21.0.0",
"react": "^16.12.0 || ^17.0.0",
"react-dom": "^16.12.0 || ^17.0.0",
"@openedx/paragon": "^22.15.1",
"react": "^16.12.0 || ^17.0.0 || ^18.0.0",
"react-dom": "^16.12.0 || ^17.0.0 || ^18.0.0",
"react-instantsearch-dom": "^6.8.3",
"react-router-dom": "^6.0.0"
}
Expand Down
7 changes: 1 addition & 6 deletions packages/catalog-search/src/CurrentRefinements.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable react-hooks/rules-of-hooks */
import React, { useContext, useMemo, useState } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
Expand All @@ -23,13 +22,9 @@ import { removeFromRefinementArray, deleteRefinementAction } from './data/action
import messages from './messages';

export const CurrentRefinementsBase = ({ items, variant }) => {
if (!items || !items.length) {
return null;
}

const [showAllRefinements, setShowAllRefinements] = useState(false);
const { refinements, dispatch } = useContext(SearchContext);
const activeRefinementsAsFlatArray = useActiveRefinementsAsFlatArray(items);
const activeRefinementsAsFlatArray = useActiveRefinementsAsFlatArray(items || []);
const intl = useIntl();

/**
Expand Down
38 changes: 14 additions & 24 deletions packages/catalog-search/src/SearchPagination.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useContext, useMemo } from 'react';
import React, { useContext } from 'react';
import PropTypes from 'prop-types';
import { connectPagination } from 'react-instantsearch-dom';
import { Pagination, Icon } from '@openedx/paragon';
import { Pagination } from '@openedx/paragon';
import { ArrowBackIos, ArrowForwardIos } from '@openedx/paragon/icons';

import { useIntl } from '@edx/frontend-platform/i18n';
Expand All @@ -16,27 +16,17 @@ export const SearchPaginationBase = ({
const { dispatch } = useContext(SearchContext);
const intl = useIntl();

const icons = useMemo(
() => ({
left: (
<>
<Icon src={ArrowBackIos} />
<div className="sr-only">Navigate Left</div>
</>
),
right: (
<>
<Icon src={ArrowForwardIos} />
<div className="sr-only">Navigate Right</div>
</>
),
}),
[],
);

const buttonLabels = {
previous: '',
next: '',
previous: intl.formatMessage({
id: 'catalog.search.pagination.previous',
defaultMessage: 'Previous',
description: 'Label for the previous button in the pagination component',
}),
next: intl.formatMessage({
id: 'catalog.search.pagination.next',
defaultMessage: 'Next',
description: 'Label for the next button in the pagination component',
}),
page: intl.formatMessage({
id: 'catalog.search.pagination.page',
defaultMessage: 'Page',
Expand Down Expand Up @@ -77,8 +67,8 @@ export const SearchPaginationBase = ({
maxPagesDisplayed={maxPagesDisplayed}
buttonLabels={buttonLabels}
icons={{
leftIcon: icons.left,
rightIcon: icons.right,
leftIcon: ArrowBackIos,
rightIcon: ArrowForwardIos,
}}
/>
);
Expand Down
3 changes: 1 addition & 2 deletions packages/catalog-search/src/data/tests/hooks.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable react/prop-types */
import { renderHook } from '@testing-library/react-hooks';
import { renderHook } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { SUBJECTS, AVAILABLILITY, FACET_ATTRIBUTES } from './constants';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { renderWithRouter } from '@edx/frontend-enterprise-utils';
import { screen, fireEvent } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { screen, act } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { IntlProvider } from '@edx/frontend-platform/i18n';
Expand Down Expand Up @@ -28,8 +29,10 @@ describe('<ClearCurrentRefinements />', () => {
const spy = jest.spyOn(actions, 'clearRefinementsAction');
renderWithRouter(<ClearCurrentRefinementsWrapper />);

// click a specific refinement to remove it
fireEvent.click(screen.queryByText(CLEAR_ALL_TEXT));
await act(async () => {
// click the clear all button
await userEvent.click(screen.queryByText(CLEAR_ALL_TEXT));
});

// assert the clicked refinement in the url is removed but others stay put
expect(spy).toHaveBeenCalledTimes(1);
Expand Down
26 changes: 17 additions & 9 deletions packages/catalog-search/src/tests/SearchBox.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ describe('<SearchBox />', () => {
});

renderWithSearchContext(<SearchBoxBase enterpriseSlug="test-enterprise" index={index} />);
userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
await act(async () => {
await userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
});
await waitFor(() => expect(index.search.mock.calls.length).toBe(1));
expect(index.search).toHaveBeenCalledWith(
'test query',
Expand All @@ -103,26 +105,32 @@ describe('<SearchBox />', () => {
renderWithSearchContext(<SearchBoxBase enterpriseSlug="test-enterprise" index={index} />);

// fill in search input and submit the search
userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
userEvent.type(screen.getByRole('searchbox'), '{enter}');
await act(async () => {
await userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
await userEvent.type(screen.getByRole('searchbox'), '{enter}');
});

// assert url is updated with the query
expect(mockedNavigator).toHaveBeenCalledWith({ pathname: '/', search: 'q=test%20query' });
// check tracking is not invoked due to absent trackingName in context
expect(sendTrackEvent).not.toHaveBeenCalled();

// clear the input
userEvent.click(screen.getByText('clear search'));
await act(async () => {
await userEvent.click(screen.getByRole('button', { name: 'clear search' }));
});

// assert query no longer exists in url
await waitFor(() => expect(mockedNavigator).toHaveBeenCalledWith({ pathname: '/', search: '' }));
});
test('tracking event when search initiated with trackingName present in context', () => {
test('tracking event when search initiated with trackingName present in context', async () => {
renderWithSearchContextAndTracking(<SearchBoxBase enterpriseSlug="test-enterprise" index={index} />, 'aProduct');

// fill in search input and submit the search
userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
userEvent.type(screen.getByRole('searchbox'), '{enter}');
await act(async () => {
await userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
await userEvent.type(screen.getByRole('searchbox'), '{enter}');
});
// check tracking is invoked due to trackingName in context
expect(sendTrackEvent).toHaveBeenCalledWith(
`${SEARCH_EVENT_NAME_PREFIX}.aProduct.${QUERY_SUBMITTED_EVENT}`,
Expand Down Expand Up @@ -154,12 +162,12 @@ describe('<SearchBox />', () => {

// fill in search input and submit the search
await act(async () => {
userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
await userEvent.type(screen.getByRole('searchbox'), TEST_QUERY);
});
await waitFor(() => expect(screen.queryByTestId('suggestions')).not.toBeNull());
await waitFor(() => expect(screen.getByText('test-title')).toBeInTheDocument());
await act(async () => {
userEvent.click(screen.getByText('test-title'));
await userEvent.click(screen.getByText('test-title'));
});
expect(suggestionSubmitOverride).toHaveBeenCalledWith(
{ learning_type: 'course', _highlightResult: { title: { value: 'test-title' } } },
Expand Down
23 changes: 16 additions & 7 deletions packages/catalog-search/src/tests/SearchPagination.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { useLocation } from 'react-router-dom';
import { screen, fireEvent } from '@testing-library/react';
import { act, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom/extend-expect';

import { SearchPaginationBase } from '../SearchPagination';
Expand All @@ -20,17 +21,20 @@ describe('<SearchPagination />', () => {
jest.clearAllMocks();
});

test('updates url when navigating right', () => {
test('updates url when paginating right', async () => {
renderWithSearchContext(<SearchPaginationBase nbPages={3} />);

// assert no initial page query parameter
expect(window.location.search).toEqual('');

// click on next button and assert page query parameter exists and is accurate
fireEvent.click(screen.queryByText('Navigate Right'));
await act(async () => {
await userEvent.click(screen.getByLabelText('Next, Page 2'));
});
expect(mockedNavigator).toHaveBeenCalledWith({ pathname: '/', search: 'page=2' });
});
test('deletes page query when navigating to the first page', () => {

test('deletes page query when navigating to the first page', async () => {
const mockedLocation = {
pathname: '/',
search: '?page=2',
Expand All @@ -44,10 +48,13 @@ describe('<SearchPagination />', () => {
expect(mockedNavigator.mock.calls[0][0]).toEqual({ pathname: '/', search: 'page=2' });

// click on prev button and assert page disappears
fireEvent.click(screen.queryByText('Navigate Left'));
await act(async () => {
await userEvent.click(screen.getByLabelText('Previous, Page 1'));
});
expect(mockedNavigator.mock.calls[1][0]).toEqual({ pathname: '/', search: '' });
});
test('updates page query when navigating left to a previous page', () => {

test('updates page query when navigating left to a previous page', async () => {
const mockedLocation = {
pathname: '/',
search: '?page=3',
Expand All @@ -62,7 +69,9 @@ describe('<SearchPagination />', () => {
expect(mockedNavigator.mock.calls[0][0]).toEqual({ pathname: '/', search: 'page=3' });

// click on prev button and assert page disappears
fireEvent.click(screen.queryByText('Navigate Left'));
await act(async () => {
await userEvent.click(screen.getByLabelText('Previous, Page 2'));
});
expect(mockedNavigator.mock.calls[1][0]).toEqual({ pathname: '/', search: 'page=2' });
});
});
14 changes: 9 additions & 5 deletions packages/catalog-search/src/tests/SearchSuggestionItem.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { renderWithRouter } from '@edx/frontend-enterprise-utils';
import React from 'react';
import { screen } from '@testing-library/react';
import { act, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import SearchSuggestionItem from '../SearchSuggestionItem';

Expand Down Expand Up @@ -61,7 +61,7 @@ describe('<SeachSuggestionItem />', () => {
expect(screen.getByText('Professional Program')).not.toBeNull();
});

test('redirects on click if disableSuggestionRedirect is false', () => {
test('redirects on click if disableSuggestionRedirect is false', async () => {
const mockData = {
program: {
url: '/test-enterprise/program/456',
Expand All @@ -83,11 +83,13 @@ describe('<SeachSuggestionItem />', () => {
hit={mockData.program.hit}
disableSuggestionRedirect={mockData.program.disableSuggestionRedirect}
/>);
userEvent.click(container.getElementsByClassName('suggestion-item')[0]);
await act(async () => {
await userEvent.click(container.getElementsByClassName('suggestion-item')[0]);
});
expect(window.location.pathname).toBe(mockData.program.url);
});

test('fires callback on click if disableSuggestionRedirect is true', () => {
test('fires callback on click if disableSuggestionRedirect is true', async () => {
const mockData = {
program: {
url: '/test-enterprise/program/456',
Expand All @@ -109,7 +111,9 @@ describe('<SeachSuggestionItem />', () => {
hit={mockData.program.hit}
disableSuggestionRedirect={mockData.program.disableSuggestionRedirect}
/>);
userEvent.click(container.getElementsByClassName('suggestion-item')[0]);
await act(async () => {
await userEvent.click(container.getElementsByClassName('suggestion-item')[0]);
});
expect(mockData.program.suggestionItemHandler).toHaveBeenCalledWith(mockData.program.hit);
});
});
Loading