Skip to content

Conversation

@fgnass
Copy link
Contributor

@fgnass fgnass commented Feb 12, 2025

This PR addresses type-related issues discussed in #4987 and paves the way to a more type-safe codebase.

What this PR does

  1. The "build:esm" script of each package now emits TypeScript declarations in dist/esm. It does this by running babel and tsc in parallel.
  2. Each package.json now has a "types" field that points to dist/esm/index.d.ts 🎉
  3. Each package now has its own tsconfig.json file that extends a common base. This allows us to use proper project references instead of treating everything as one big project.
  4. It fixes a circular type dependency (stega.ts was in the wrong package) and adds missing types for the semaphore npm package.

What this PR does not do

This PR does not change any of the handwritten types defined in decap-cms-core. It only converts the /index.d.ts file into src/types/index.ts.

Moving forward from here

The plan is to create a follow-up PR (once the new system is in place) that will replace the handwritten public types with the internal ones. With this PR as foundation, it will also be much easier to gradually convert more and more .js files into .ts files.

I would love to hear your feedback and if you think that this is the right way to tackle this.

@fgnass fgnass requested a review from a team as a code owner February 12, 2025 17:23
import { connect } from 'react-redux';
import { encodeEntry } from 'decap-cms-lib-util/src/stega';

import { encodeEntry } from '../../../lib/stega';
Copy link

Choose a reason for hiding this comment

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

Thanks, this will fix #7401!

One thing might be worth doing is to add an alias to each tsconfig.json, e.g.

"compilerOptions": {
  "paths": {
    "@/*": ["./src/*"]
  }

then here you can use the alias path...

import { encodeEntry } from '@/lib/stega';

Personally, I find this easier to rationalize the file structure, when importing higher up the tree. This is definitely not required, but something to consider.

Thanks again for your awesome contributions!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @eoneill, I was not aware that this was causing runtime errors! I created a separate PR (#7405) to address this in isolation.

@fgnass
Copy link
Contributor Author

fgnass commented Feb 25, 2025

Closing this in favor of #7411

@fgnass fgnass closed this Feb 25, 2025
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