Skip to content

Conversation

@belanglos
Copy link
Contributor

@belanglos belanglos commented Oct 10, 2025

Summary

This PR

  • introduces a new shared folder /shared with ts-code. Some of that code has been copied over from /domain
  • has changes to our pr and ref-pipelines
    • previously, we've run npm test in a docker container, which is not necessary, the tests are now run on the agent
    • previously, we had integration-tests in place, but they were not up-to-date so they have been removed from the ref/pr pipelines. A task to deal with them has been created here
  • contains changes to the build process, the reason for that being that the shared folder has to be compiled before the server-folder can run it.
    • this includes changes to nodemon to make sure we have hot-reload in place
  • contains an updated and cleaner jest-setup, with one file with configs that is shared between specific files for each code-folder. This ensures that only test-files (but all of them) are picked up and that modules can be resolved correctly
  • removes formatISODate which is not used anymore. The initial reason for removal as part of this PR was that the tests behaved differently in the pipeline (on-agent) compared to on the local machine. (because of time-zones)

Jest changes

jest.base.js // shared config
jest.config.js // root config that points out the other jest configs
jest.client.config.js // jest configs for respective project
jest.server.config.js
jest.domain.config.js
jest.shared.config.js // end

Testing

  • extensive local testing has been performed
  • both pr and ref-pipelines work (tested in azure)
  • the resulting app has been smoke-tested ✅
  • manually clicky-tested ✅
  • cypress-tests have been run ✅

Copilot generated summary

This pull request introduces significant improvements to the build, test, and module resolution workflow, refactors scripts for better reliability, and removes unused code. The main changes focus on adopting a TypeScript build step for shared code, updating test configurations to support multi-project Jest setups, and cleaning up module imports and exports for maintainability.

Build and Test Pipeline Modernization

  • Updated Azure Pipelines configuration to run unit tests using an npm-based template (npm.yml) instead of the previous Docker Compose setup, specifying Node.js 20 and a shared build step. This streamlines CI and aligns with the new TypeScript build step. (.azure/azure-pipelines.pr.yml, .azure/azure-pipelines.ref.yml) [1] [2] [3]

  • Added a TypeScript build step for the shared codebase (build:shared), and updated package.json scripts to ensure shared code is built before running builds or tests. This improves reliability and developer experience.

Multi-Project Jest Configuration

  • Introduced a base Jest configuration (jest.base.js) and separate configs for client, server, domain, and shared code, enabling targeted and efficient testing across different parts of the codebase. New scripts allow running tests per project or only on changed files. [1] [2] [3] [4] [5] [6]

  • Added ts-jest and improved module mapping in test configs to support TypeScript and cross-project imports.

Module Resolution and Import Clean-up

  • Updated TypeScript configuration (public/tsconfig.json) to support path aliases for shared and domain code, and added project references for incremental builds.

  • Improved ESLint configuration to support webpack-based import resolution, aiding editor integration and code quality.

  • Updated imports in several files to use new path aliases and shared code, supporting code reuse and maintainability. [1] [2] [3] [4] [5]

Script and Environment Improvements

  • Refactored build.sh for better reliability: added strict error handling, made the environment argument optional, and improved conditional logic for build modes. [1] [2] [3] [4]

  • Enhanced nodemon.json to watch additional folders and set environment variables for development.

Code and Test Clean-up

  • Removed the unused formatISODate function and its associated tests from the domain/date.js module, reflecting a move away from legacy date formatting logic. [1] [2]

These changes collectively modernize the development workflow, improve build and test reliability, and pave the way for easier maintenance and future enhancements.

@belanglos belanglos force-pushed the issues/KP-448-lyft-kod-i-shared-som-ts branch 5 times, most recently from 613845a to 0829147 Compare October 16, 2025 12:30
@belanglos belanglos marked this pull request as ready for review October 16, 2025 13:25
@belanglos belanglos requested a review from Copilot October 16, 2025 14:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new TypeScript-enabled shared code directory and modernizes the test and build infrastructure. The main purpose is to enable better code sharing across different parts of the application by creating a /shared folder with TypeScript support, while also improving the CI/CD pipeline and Jest configuration for better development experience.

Key changes include:

  • Introduction of a new /shared TypeScript directory with utilities moved from /domain
  • Migration from Docker-based to npm-based test execution in CI pipelines
  • Comprehensive Jest configuration refactoring with multi-project setup

Reviewed Changes

Copilot reviewed 31 out of 34 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
shared/ New TypeScript modules for term handling, semester utilities, and periods
webpack.config.js Added TypeScript support and path aliases for shared/domain imports
package.json Added build scripts for shared code and Jest project configurations
jest.*.config.js Multi-project Jest setup with separate configs for client/server/domain/shared
.azure/ Updated CI pipelines to use npm-based testing instead of Docker
build.sh Improved script reliability with error handling and optional environment parameter

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@belanglos belanglos force-pushed the issues/KP-448-lyft-kod-i-shared-som-ts branch from 0829147 to 1ea68d0 Compare October 17, 2025 06:35
@belanglos belanglos requested a review from Copilot October 17, 2025 06:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 31 out of 34 changed files in this pull request and generated 5 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@allazis allazis left a comment

Choose a reason for hiding this comment

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

Really good job on this Benni! ⭐

For the semesterUtils and other utils files, would it be possible to add some example input and outputs in the function description? Simply done as for

/**
 *
 * @param kthSemesterString 20241
 * @returns ["2024", "1"]
 */

for example. It's tricky to keep the different period, semester and term formats in your head.

@belanglos
Copy link
Contributor Author

belanglos commented Oct 21, 2025

For the semesterUtils and other utils files, would it be possible to add some example input and outputs in the function description? Simply done as for

/**
 *
 * @param kthSemesterString 20241
 * @returns ["2024", "1"]
 */

for example. It's tricky to keep the different period, semester and term formats in your head.

Thank you for pointing this out, @allazis , I whole-heartedly agree! This is now addressed in 4881d0e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants