-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat: improve react-18 tests #25758
feat: improve react-18 tests #25758
Changes from all commits
25cff03
5412278
cba9aba
a44c913
35c95fd
64e8615
84e0eab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,5 @@ | ||
{ | ||
"extends": ["plugin:@fluentui/eslint-plugin/react--legacy"], | ||
"extends": ["plugin:@fluentui/eslint-plugin/react"], | ||
"root": true, | ||
"rules": { | ||
"deprecation/deprecation": "off", | ||
"prefer-const": "off" | ||
} | ||
"rules": {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
// @ts-check | ||
|
||
const { resolveMergeStylesSerializer } = require('@fluentui/scripts/jest/jest-resources'); | ||
const getResolveAlias = require('@fluentui/scripts/webpack/getResolveAlias'); | ||
|
||
/** | ||
* @type {import('@jest/types').Config.InitialOptions} | ||
*/ | ||
|
@@ -9,12 +12,18 @@ module.exports = { | |
globals: { | ||
'ts-jest': { | ||
tsConfig: '<rootDir>/tsconfig.spec.json', | ||
diagnostics: false, | ||
diagnostics: { warnOnly: true, exclude: ['packages/**'] }, | ||
}, | ||
}, | ||
transform: { | ||
'^.+\\.tsx?$': 'ts-jest', | ||
}, | ||
|
||
coverageDirectory: './coverage', | ||
setupFilesAfterEnv: ['./config/tests.js'], | ||
moduleNameMapper: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. previous setup was using path mappings for v9 which is has no effect |
||
'\\.(scss)$': '@fluentui/scripts/jest/jest-style-mock.js', | ||
...getResolveAlias(), | ||
}, | ||
snapshotSerializers: [resolveMergeStylesSerializer()], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,27 @@ | ||
{ | ||
"name": "@fluentui/react-18-tests-v8", | ||
"description": "React 18 test application", | ||
"version": "1.0.0", | ||
"version": "8.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is needed as tooling checks version and tweaks its behaviours based on that ( cypress ) |
||
"private": true, | ||
"scripts": { | ||
"build": "just-scripts build", | ||
"clean": "just-scripts clean", | ||
"code-style": "just-scripts code-style", | ||
"type-check": "tsc -b tsconfig.json", | ||
"format": "prettier -w . --ignore-path ../.prettierignore", | ||
"format:check": "yarn format -c", | ||
"e2e": "cypress run --component", | ||
"e2e:local": "cypress open --component", | ||
"lint": "just-scripts lint", | ||
"test": "just-scripts test", | ||
"just": "just-scripts", | ||
"start": "webpack-dev-server --mode=development --config=webpack.config.js", | ||
"type-check": "tsc -b tsconfig.json" | ||
"lint": "eslint --ext .js,.ts.,.tsx ./src", | ||
"test": "jest --passWithNoTests", | ||
"start": "webpack-dev-server --mode=development" | ||
}, | ||
"devDependencies": { | ||
"@fluentui/eslint-plugin": "*", | ||
"@fluentui/scripts": "^1.0.0", | ||
"@types/react": "18.0.14", | ||
"@types/react-dom": "18.0.6", | ||
"swc-loader": "^0.2.3" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. swc-loader is in root package version already - single version policy conformance |
||
"@fluentui/scripts": "^1.0.0" | ||
}, | ||
"dependencies": { | ||
"@fluentui/react": "^8.103.0", | ||
"@fluentui/react-hooks": "^8.6.14", | ||
"@types/react": "18.0.14", | ||
"@types/react-dom": "18.0.6", | ||
"react": "18.2.0", | ||
"react-dom": "18.2.0", | ||
"tslib": "^2.1.0" | ||
|
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import * as React from 'react'; | ||
import { mount } from '@cypress/react'; | ||
|
||
import { ContextualMenuExample } from './ContextualMenuExample'; | ||
|
||
const menuTriggerSelector = '[type="button"]'; | ||
const menuSelector = '[role="menu"]'; | ||
|
||
describe('ContextualMenu in React 18', () => { | ||
it('renders ContextualMenu when trigger button is clicked', () => { | ||
mount( | ||
<React.StrictMode> | ||
<ContextualMenuExample /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this uses implementation instead of having the same code on 3 places |
||
</React.StrictMode>, | ||
); | ||
|
||
cy.get(menuTriggerSelector).click().get(menuSelector).should('be.visible'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import * as React from 'react'; | ||
import { render } from '@testing-library/react'; | ||
import userEvent from '@testing-library/user-event'; | ||
|
||
import { ContextualMenuExample } from './ContextualMenuExample'; | ||
|
||
describe('ContextualMenu in React 18', () => { | ||
// eslint-disable-next-line @typescript-eslint/no-empty-function | ||
const noop = () => {}; | ||
|
||
beforeEach(() => { | ||
jest.spyOn(console, 'warn').mockImplementation(noop); | ||
}); | ||
|
||
it('should render ContextualMenu when trigger button is clicked', () => { | ||
const { getByRole, queryAllByRole } = render( | ||
<React.StrictMode> | ||
<ContextualMenuExample /> | ||
</React.StrictMode>, | ||
); | ||
|
||
expect(queryAllByRole('menu').length).toBe(0); | ||
|
||
const menuTrigger = getByRole('button'); | ||
userEvent.click(menuTrigger); | ||
|
||
expect(queryAllByRole('menu').length).toBe(1); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,8 @@ | |
"declaration": true, | ||
"declarationDir": "dist/types", | ||
"inlineSources": true, | ||
"types": ["static-assets", "environment", "node"] | ||
"types": ["static-assets", "environment"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. node types should not be present in web app |
||
}, | ||
"exclude": ["./src/common/**", "**/*.spec.ts", "**/*.spec.tsx", "**/*.test.ts", "**/*.test.tsx"], | ||
"exclude": ["**/*.spec.ts", "**/*.spec.tsx", "**/*.test.ts", "**/*.test.tsx", "**/*.cy.ts", "**/*.cy.tsx"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated to follow our approach with |
||
"include": ["./src/**/*.ts", "./src/**/*.tsx"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
const path = require('path'); | ||
|
||
const HtmlWebpackPlugin = require('html-webpack-plugin'); | ||
const { TsconfigPathsPlugin } = require('tsconfig-paths-webpack-plugin'); | ||
|
||
const getResolveAlias = require('@fluentui/scripts/webpack/getResolveAlias'); | ||
|
||
module.exports = () => { | ||
return { | ||
|
@@ -12,22 +12,53 @@ module.exports = () => { | |
}, | ||
resolve: { | ||
extensions: ['.tsx', '.ts', '.js'], | ||
plugins: [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this had no use as it would map only v9 packages |
||
new TsconfigPathsPlugin({ | ||
configFile: path.resolve(__dirname, '../../tsconfig.base.json'), | ||
}), | ||
], | ||
alias: { | ||
react: path.resolve(__dirname, './node_modules/react'), | ||
'react-dom': path.resolve(__dirname, './node_modules/react-dom'), | ||
...getResolveAlias(), | ||
}, | ||
}, | ||
module: { | ||
rules: [ | ||
{ | ||
test: /\.tsx?$/, | ||
exclude: /node_modules/, | ||
use: 'swc-loader', | ||
use: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. v8 cannot be compiled with swc, switching to esbuild for now swc-project/swc#2037 |
||
loader: 'esbuild-loader', | ||
options: { | ||
loader: 'tsx', | ||
target: 'es2019', | ||
}, | ||
}, | ||
}, | ||
{ | ||
test: /\.scss$/, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are sass and css loaders only added now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. previously v8 needed to build everything prior to run. also it was misconfigured to use v9 path aliases etc. more info is in PR description |
||
enforce: 'pre', | ||
exclude: [/node_modules/], | ||
use: [ | ||
{ | ||
loader: '@microsoft/loader-load-themed-styles', // creates style nodes from JS strings | ||
}, | ||
{ | ||
loader: 'css-loader', // translates CSS into CommonJS | ||
options: { | ||
esModule: false, | ||
modules: true, | ||
importLoaders: 2, | ||
}, | ||
}, | ||
{ | ||
loader: 'postcss-loader', | ||
options: { | ||
postcssOptions: { | ||
plugins: ['autoprefixer'], | ||
}, | ||
}, | ||
}, | ||
{ | ||
loader: 'sass-loader', | ||
}, | ||
], | ||
}, | ||
], | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,5 @@ | ||
{ | ||
"extends": ["plugin:@fluentui/eslint-plugin/react--legacy"], | ||
"extends": ["plugin:@fluentui/eslint-plugin/react"], | ||
"root": true, | ||
"rules": { | ||
"deprecation/deprecation": "off", | ||
"prefer-const": "off" | ||
} | ||
"rules": {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this exposed lot of warnings from v8 packages - caused by not following isolateModules.
this will resolve it #25774