stackblitz / tutorialkit

TutorialKit by StackBlitz - Create interactive tutorials powered by the WebContainer API
https://tutorialkit.dev
MIT License
460 stars 43 forks source link

chore: remove spec files from runtime package #236

Closed PhearZero closed 2 months ago

PhearZero commented 2 months ago

Overview

Looking at the package, everything seemed like it should not include the spec files. After some investigation I found some strange behavior with the components package being included in the build 🤔.

Summary

Package does not respect the tsconfig.build.json when @tutorialkit/components-react is included in the --filter.

Fix

The quick and dirty fix is to just exclude the @tutorial/components-react from the filter and have a two step build. It may need further triage with pnpm/typescript. Happy to dig deeper!

stackblitz[bot] commented 2 months ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

AriPerkkio commented 2 months ago

Thanks for looking into this!

Package does not respect the tsconfig.build.json when @tutorialkit/components-react is included in the --filter.

Oh wow, this is the case indeed. As soon as @tutorialkit/components-react is dropped from build script, test files are excluded. This seems to happen even in https://github.com/stackblitz/tutorialkit/pull/155 where the package was moved to packages/react, so it seems this is not caused by nested directories.

It would be great to figure out why exactly this is happening. 🤔

PhearZero commented 2 months ago

Anytime! Thank you all for the project ❤️. I was shocked as well!

It would be great to figure out why exactly this is happening. 🤔

If someone doesn't get to it, I'll try to dig in this evening (it's going to be non-obvious). My first instinct is to dig into pnpm v8.15.6. I'll report back here with any findings 🕵️

Nemikolh commented 2 months ago

Hey @PhearZero, thanks for investigating! This is indeed really odd.

I wonder if this is a bug with the fact that the package is marked as composite but project reference default to the tsconfig.json. I think if we modify the { "path": "..." } references to point to the tsconfig.build.json it will work?

PhearZero commented 2 months ago

Thanks for the hint @Nemikolh ❤️, I was able to update the references for the runtime to point to the tsconfig.build.json and that seems to have resolved the build!

PhearZero commented 2 months ago

@Nemikolh Makes total sense, I've updated the the references to point to the build configurations

AriPerkkio commented 2 months ago

@PhearZero the CI is failing on lint step. Could you run pnpm run lint --fix and commit the changes.

Or check the Allow edits and access to secrets by maintainers for this PR and I'll push the required changes.

PhearZero commented 2 months ago

@AriPerkkio it looks like edits by maintainer is not supported in a fork from another organization

I was able to use the Update branch button to merge in main

AriPerkkio commented 2 months ago

Oh right, you forked TutorialKit repo to another Github organization instead of personal account. That makes sense.

Thanks for the fix!