❓ can we just ship typescript

jeremybanka commented 1 month ago

PR Type

enhancement, configuration changes, dependencies

Description

Removed define-integration-scope.ts script.
Updated package exports and submodule manifests to use src instead of dist.
Removed tsup.config.ts files for various packages.
Updated main tsup.config.ts to use src instead of dist.
Removed references to tsconfig.prod.json and updated vitest.config.ts to use only development tsconfig.
Updated package.json files to use src instead of dist and removed individual build scripts for submodules.

Changes walkthrough 📝

Relevant files

Enhancement

5 files

define-integration-scope.ts Remove `define-integration-scope.ts` script. packages/atom.io/__scripts__/define-integration-scope.ts - Removed the entire file.	+0/-99
define-package-exports.ts Update package exports to use `src` instead of `dist`. packages/atom.io/__scripts__/define-package-exports.ts Updated `files` and `exports` fields to use `src` instead of `dist`. Added special handling for `eslint-plugin` submodule.	+13/-11
define-submodule-manifests.ts Update submodule manifests to use `src` instead of `dist`. packages/atom.io/__scripts__/define-submodule-manifests.ts Updated submodule manifests to use `src` instead of `dist`. Added special handling for `eslint-plugin` submodule.	+15/-11
manifest-build.node.ts Remove `define-integration-scope` from build manifest script. packages/atom.io/__scripts__/manifest-build.node.ts - Removed import and usage of `define-integration-scope`.	+0/-2
manifest-test.node.ts Remove `define-integration-scope` from test manifest script. packages/atom.io/__scripts__/manifest-test.node.ts - Removed import and usage of `define-integration-scope`.	+0/-2

Configuration changes

33 files

tsup.config.ts Remove `tsup.config.ts` for data package. packages/atom.io/data/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for ephemeral package. packages/atom.io/ephemeral/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for eslint-plugin package. packages/atom.io/eslint-plugin/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for immortal package. packages/atom.io/immortal/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for internal package. packages/atom.io/internal/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for introspection package. packages/atom.io/introspection/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for react-devtools package. packages/atom.io/react-devtools/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for react package. packages/atom.io/react/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for realtime-client package. packages/atom.io/realtime-client/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for realtime-react package. packages/atom.io/realtime-react/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for realtime-server package. packages/atom.io/realtime-server/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for realtime-testing package. packages/atom.io/realtime-testing/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for realtime package. packages/atom.io/realtime/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Remove `tsup.config.ts` for transceivers/set-rtx package. packages/atom.io/transceivers/set-rtx/tsup.config.ts - Removed the entire file.	+0/-5
tsup.config.ts Update main `tsup.config.ts` to use `src` instead of `dist`. packages/atom.io/tsup.config.ts Removed `DTS_OPTIONS` configuration. Updated `JS_OPTIONS` to use `src` instead of `dist`.	+1/-25
vitest.config.ts Update `vitest.config.ts` to use only development tsconfig. packages/atom.io/vitest.config.ts - Removed reference to `tsconfig.prod.json`.	+1/-3
package.json Update data package.json to use `src` instead of `dist`. packages/atom.io/data/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update ephemeral package.json to use `src` instead of `dist`. packages/atom.io/ephemeral/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update eslint-plugin package.json to use `src` instead of `dist`. packages/atom.io/eslint-plugin/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+2/-6
package.json Update immortal package.json to use `src` instead of `dist`. packages/atom.io/immortal/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update internal package.json to use `src` instead of `dist`. packages/atom.io/internal/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update introspection package.json to use `src` instead of `dist`. packages/atom.io/introspection/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update json package.json to use `src` instead of `dist`. packages/atom.io/json/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update main package.json to use `src` instead of `dist`. packages/atom.io/package.json Removed `dist` references from `files` and `exports` fields. Updated `main` and `exports` fields to use `src` instead of `dist`. Removed individual build scripts for submodules.	+32/-97
package.json Update react-devtools package.json to use `src` instead of `dist`. packages/atom.io/react-devtools/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update react package.json to use `src` instead of `dist`. packages/atom.io/react/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update realtime-client package.json to use `src` instead of `dist`. packages/atom.io/realtime-client/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update realtime-react package.json to use `src` instead of `dist`. packages/atom.io/realtime-react/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update realtime-server package.json to use `src` instead of `dist`. packages/atom.io/realtime-server/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update realtime-testing package.json to use `src` instead of `dist`. packages/atom.io/realtime-testing/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json Update realtime package.json to use `src` instead of `dist`. packages/atom.io/realtime/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
package.json `Update transceivers/set-rtx package.json to use src instead of dist.` packages/atom.io/transceivers/set-rtx/package.json Updated `main` and `exports` fields to use `src` instead of `dist`.	+3/-7
tsconfig.prod.json Remove `tsconfig.prod.json`. packages/atom.io/tsconfig.prod.json - Removed the entire file.	+0/-62

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
atom-io-fyi	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 4, 2024 9:19pm
wayfarer-quest	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 4, 2024 9:19pm

changeset-bot[bot] commented 1 month ago

⚠️ No Changeset found

Latest commit: 9f9cf137f052770e05e415f91f1aae5d49e98869

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

**Possible Bug:** The PR removes all references to the `dist` directory and replaces them with `src`. This could lead to issues where the production build expects compiled files which are typically in `dist`. Ensure that the build and deployment processes are updated to handle source files directly if that is the intent. **Refactoring Scope:** The removal of multiple `tsup.config.ts` files suggests a significant change in how builds are managed. It's crucial to verify that the remaining build configurations cover all necessary cases previously handled by the individual configurations. **Dependency Management:** The removal of `tsconfig.prod.json` and changes in package.json files across multiple packages could affect the project's dependency resolution and build process. Thorough testing is needed to ensure that these changes do not break the dependency tree and module resolution.

github-actions[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Correct the 'types' field to point to a TypeScript declaration file ___ Ensure that the 'types' field points to a TypeScript declaration file (`.d.ts`) instead of a TypeScript source file to adhere to common practices and improve tooling support. [packages/atom.io/__scripts__/define-submodule-manifests.ts [124-127]](https://github.com/jeremybanka/wayforge/pull/2217/files#diff-c5a355888daa04ac0caeaef95c65d849be9cf837cc5728429b03f6821b264ad1R124-R127) ```diff ".": { import: `./src/index.ts`, - types: `./src/index.ts`, + types: `./src/index.d.ts`, # Corrected to point to a declaration file } ``` Suggestion importance[1-10]: 10 Why: Pointing the 'types' field to a TypeScript declaration file is a best practice that enhances tooling support and is crucial for proper type checking. The suggestion is correct and highly beneficial.	10
Possible issue	Correct the file extension for TypeScript type definitions ___ The 'types' field should point to a TypeScript declaration file (typically .d.ts) instead of a TypeScript source file. This change ensures that the package correctly exposes type definitions. [packages/atom.io/realtime-react/package.json [9]](https://github.com/jeremybanka/wayforge/pull/2217/files#diff-a293cd3e82a5cbf44269bbc59bc0a9ec37ac91b85d9911881ee1e7c7343e8e98R9-R9) ```diff -"types": "./src/index.ts" +"types": "./src/index.d.ts" ``` Suggestion importance[1-10]: 10 Why: The suggestion correctly identifies that the 'types' field should point to a TypeScript declaration file (.d.ts) instead of a TypeScript source file (.ts), which is crucial for proper type definition exposure in the package.	10
Enhancement	Add a 'require' field to the main package export for CommonJS compatibility ___ Consider adding a 'require' field for the main package export to ensure compatibility with CommonJS modules, similar to how it's defined for the 'eslint-plugin' submodule. [packages/atom.io/__scripts__/define-package-exports.ts [42-45]](https://github.com/jeremybanka/wayforge/pull/2217/files#diff-a2b71d0532fdf8770a1d33c61472b67fe0c35c2aa3071ba4593ec9a0b9bf8c49R42-R45) ```diff ".": { types: `./src/index.ts`, import: `./src/index.ts`, + require: `./src/index.cjs`, # Assuming a compiled CommonJS entry exists } ``` Suggestion importance[1-10]: 9 Why: This suggestion improves compatibility with CommonJS modules, which is important for broader usage and integration. The suggestion is accurate and contextually appropriate.	9
Enhancement	Add a 'types' field to the 'eslint-plugin' submodule export ___ For the 'eslint-plugin' submodule, add a 'types' field to specify the TypeScript declaration file, enhancing editor support and type checking for users of the package. [packages/atom.io/__scripts__/define-package-exports.ts [154-157]](https://github.com/jeremybanka/wayforge/pull/2217/files#diff-a2b71d0532fdf8770a1d33c61472b67fe0c35c2aa3071ba4593ec9a0b9bf8c49R154-R157) ```diff "./eslint-plugin": { import: `./eslint-plugin/dist/index.js`, require: `./eslint-plugin/dist/index.cjs`, + types: `./eslint-plugin/dist/index.d.ts`, # Added types field } ``` Suggestion importance[1-10]: 8 Why: Adding a 'types' field improves editor support and type checking for users. This is a valuable enhancement, though not as critical as the previous suggestions.	8
Maintainability	Refactor the conditional logic in the reduce function for clarity and maintainability ___ Refactor the conditional structure inside the reduce function for better readability and maintainability by separating the condition into a more explicit block. [packages/atom.io/__scripts__/define-package-exports.ts [48-57]](https://github.com/jeremybanka/wayforge/pull/2217/files#diff-a2b71d0532fdf8770a1d33c61472b67fe0c35c2aa3071ba4593ec9a0b9bf8c49R48-R57) ```diff -if (folder === `eslint-plugin`) { - acc[`./${folder}`] = { - import: `./${folder}/dist/index.js`, - require: `./${folder}/dist/index.cjs`, - } -} else { - acc[`./${folder}`] = { - types: `./${folder}/src/index.ts`, - import: `./${folder}/src/index.ts`, - } -} +acc[`./${folder}`] = folder === `eslint-plugin` + ? { + import: `./${folder}/dist/index.js`, + require: `./${folder}/dist/index.cjs`, + } + : { + types: `./${folder}/src/index.ts`, + import: `./${folder}/src/index.ts`, + }; ``` Suggestion importance[1-10]: 7 Why: The refactoring improves code readability and maintainability, which is beneficial for future development. However, it is a minor enhancement compared to the functional improvements suggested earlier.	7

coveralls commented 1 month ago

coverage: 93.985%. remained the same when pulling f27b617d614c557a9c7ac5af65d4420d53f33ec3 on atom.io-zero-build into f17a950ff4938f9616b5c95c890269babba20080 on main.

coveralls commented 1 month ago

coverage: 93.985%. remained the same when pulling 3685ad3b6b92d2f293fdb4bbe29e789bd52a4d4e on atom.io-zero-build into f17a950ff4938f9616b5c95c890269babba20080 on main.

coveralls commented 1 month ago

coverage: 93.712% (-0.3%) from 93.985% when pulling 9f9cf137f052770e05e415f91f1aae5d49e98869 on atom.io-zero-build into f17a950ff4938f9616b5c95c890269babba20080 on main.

jeremybanka / wayforge