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

Improve type safety and code quality #383

Merged
merged 11 commits into from Nov 21, 2023
Merged

Improve type safety and code quality #383

merged 11 commits into from Nov 21, 2023

Conversation

bryanjtc
Copy link
Member

@bryanjtc bryanjtc commented Nov 10, 2023

Description

This pull request introduces several changes to the codebase, primarily in the src/csf and src/playwright directories. These changes aim to improve the code quality and functionality of the project.

Changes in src/config/jest-playwright.ts

  • New Import: import type { Config } from '@jest/types'.
  • Modification: TEST_RUNNER_PATH now uses the nullish coalescing operator ?? instead of ||.
  • Update: getJestPlaywrightConfig explicitly declares return type as Config.InitialOptions.
  • Refactoring: Replaced string concatenation with template literals for path definitions.
  • Enhancement in getJestConfig function:
    • Explicitly declared return type as Config.InitialOptions.
    • Utilized optional chaining and nullish coalescing for testMatch.
    • Introduced const config with type Config.InitialOptions.

Changes in Snapshot Files

src/csf/__snapshots__/transformCsf.test.ts.snap

  • New Snapshot Test: For transformCsf handling cases with no stories.

Changes in src/csf/transformCsf.test.ts

  • New Test Case: 'returns empty result if there are no stories'.

Modifications in src/csf/transformCsf.ts

  • Type Changes: In makePlayTest and makeDescribe, return type altered to t.ExpressionStatement[].
  • Function Update: transformCsf
    • Adjusted default parameters.
    • Generic Record type used in storyPlays.
    • Altered playTests to exclude null values.

Changes in src/playwright/hooks.ts

  • Type Modification: HttpHeaderSetter to return Promise<Record<string, string>>.

Adjustments in src/playwright/index.ts

  • Function Simplification: process function signature streamlined by removing config parameter.

Updates in src/playwright/transformPlaywright.test.ts

  • Snapshot Serializer Improvement: in expect.addSnapshotSerializer.

Revisions in src/playwright/transformPlaywright.ts

  • Function Update: testPrefixer with enhanced type safety and template literals usage.
  • Refactoring: makeTitleFactory uses template literals for filePath.

Modifications in src/playwright/transformPlaywrightJson.test.ts

  • Enhanced Testing: For transformPlaywrightJson with type satisfactions.
  • New Test Case: For handling unsupported index versions.
  • New Test Case: For handling empty statements.

Changes in src/playwright/transformPlaywrightJson.ts

  • Type and Logic Updates: To handle version-specific processing in transformPlaywrightJson.

Adjustments in src/setup-page.ts

  • Function Refactoring: sanitizeURL with template literals and improved logic.
  • Simplification: In setupPage with nullish coalescing.

Changes in src/test-storybook.ts

  • Code Refactoring: For readability and efficiency.

Modifications in TypeScript Definitions (src/typings.d.ts)

  • Type Enhancements: New imports and updated global variable types.

Revisions in CLI Options Utilities (src/util/getCliOptions.*)

  • Streamlined Processing: Of CLI options and improved type clarity.

Changes in src/util/getStorybookMain.test.ts

  • Import Changes: Added storybookMainConfig to the existing imports.
  • New Test Added:
    • Test to ensure that getStorybookMain returns the configDir value from storybookMainConfig if it exists.

Changes in src/util/getStorybookMain.ts

  • Export Added: storybookMainConfig Map is now exported.
  • getStorybookMain Function Changes:
    • Modified to check storybookMainConfig for 'configDir' key instead of configDir argument.
    • Updated setting and getting logic in storybookMainConfig to use 'configDir' as a constant key.

Changes in .storybook/main.ts

  • Modification in features Configuration:
    • Update in storyStoreV7:
      • Previous Behavior: Set to false if process.env.STORY_STORE_V7 equals 'false', otherwise true.
      • New Behavior: Now true unless process.env.STORY_STORE_V7 is explicitly 'false'.

Changes in .storybook/test-runner.ts

  • Enhancement in Test Runner Configuration:
    • Refinement: Added optional chaining (?.) to innerHTML method call on elementHandler.
    • Benefit: Improves code robustness by handling potential null or undefined values for elementHandler.

Changes in test-runner-jest.config.js

  • Documentation Enhancement:
    • Addition: JSDoc comment block above the module exports.
    • Purpose: Specifies the type of the exported configuration as import('@jest/types').Config.InitialOptions.
    • Impact: Enhances code readability and ensures type safety by explicitly defining the configuration structure.

These changes collectively enhance the codebase by improving type safety, code readability, and adherence to coding standards. The introduction of new tests and error handling ensures more robust functionality.

From: #323

📦 Published PR as canary version: 0.15.3--canary.383.049359f.0

✨ Test out this PR locally via:

npm install @storybook/test-runner@0.15.3--canary.383.049359f.0
# or 
yarn add @storybook/test-runner@0.15.3--canary.383.049359f.0

Version

Published prerelease version: v0.15.3-next.0

Changelog

🐛 Bug Fix

Authors: 2

- Update test snapshots and test files in the `csf` and `playwright` directories.
- Update `hooks.ts`, `index.ts`, `transformPlaywright.test.ts`, `transformPlaywright.ts`, `transformPlaywrightJson.test.ts`, `transformPlaywrightJson.ts`, `setup-page.ts`, `test-storybook.ts`, `getCliOptions.test.ts`, `getCliOptions.ts`, `getParsedCliOptions.test.ts`, `getStorybookMain.ts`, `getStorybookMetadata.ts`, `getTestRunnerConfig.test.ts`, `getTestRunnerConfig.ts`, and `tsconfig.json`.
- These changes were made to improve the codebase and ensure compatibility with the latest dependencies and standards.
@bryanjtc bryanjtc self-assigned this Nov 10, 2023
@bryanjtc bryanjtc changed the base branch from next to enable-strict-mode November 10, 2023 21:30
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (enable-strict-mode@1843bee). Click here to learn what that means.

Additional details and impacted files
@@                  Coverage Diff                  @@
##             enable-strict-mode     #383   +/-   ##
=====================================================
  Coverage                      ?   92.05%           
=====================================================
  Files                         ?       15           
  Lines                         ?      277           
  Branches                      ?       72           
=====================================================
  Hits                          ?      255           
  Misses                        ?       22           
  Partials                      ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bryanjtc and others added 10 commits November 10, 2023 16:51
…ansformCsf.ts, src/playwright/transformPlaywrightJson.ts, and src/typings.d.ts

- Update package.json to include new dependencies or update existing ones.
- Modify src/config/jest-playwright.ts to configure Jest with Playwright.
- Refactor src/csf/transformCsf.ts to improve code readability and maintainability.
- Update src/playwright/transformPlaywrightJson.ts to handle new Playwright JSON format.
- Update src/typings.d.ts to include new type definitions or modify existing ones.
The __getContext variable was no longer being used in the codebase, so it was removed to improve code cleanliness and maintainability.
- Updated the `storyStoreV7` feature in the Storybook configuration to use a more concise expression.
- Modified the test runner to handle cases where the element handler is null or undefined.
- Added JSDoc type annotation for the exported Jest configuration.
- Refactored getStorybookMain.test.ts and getStorybookMain.ts files.
- Made improvements to the utility functions for better performance and readability.
Simplified the logic in the `transformPlaywrightJson.ts` file to improve readability and maintainability.

- Replaced the double negation with a single negation in the `makeTest` function.
- Made the `tags` property optional in the `V4Entry` type.
- Added a default value of `false` for the `metaOrStoryPlay` parameter in the `makeTest` function.
- Removed the unused import of `TestRunnerConfig` in `test-storybook.ts`.
This commit includes changes to the transformPlaywrightJson module and its corresponding test file. The purpose of these changes is to refactor the code and improve its overall structure and readability.
@yannbf yannbf changed the base branch from enable-strict-mode to next November 21, 2023 11:36
@yannbf yannbf added the patch Increment the patch version when merged label Nov 21, 2023
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Thank you so much! Great stuff 🚀

Copy link

🚀 PR was released in v0.16.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants