Skip to content

feat: template inheritance #99

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

Merged
merged 12 commits into from
Jun 27, 2024
Merged

feat: template inheritance #99

merged 12 commits into from
Jun 27, 2024

Conversation

Nemikolh
Copy link
Member

@Nemikolh Nemikolh commented Jun 26, 2024

This PR makes it possible to extend a template with another template by adding a configuration file, .tk-config.json with the following content:

{
  "extends": "../path-to-other-template"
}

So with the following structure:

templates
├── vite-app
│   ├── index.html
│   ├── main.js
│   ├── package.json
│   ├── package-lock.json
│   └── style.css
└── vite-app-2
    ├── index.html
    └── .tk-config.json

The content of vite-app-2 and vite-app will be identical except for the index.html which will be the one living in vite-app.

Notes

  • Because this is a path, it's also possible to do something like ../../node_modules/tuto-templates/foo. So this means that if you have fixed templates that lives somewhere else, you can extend them. They won't be watched by the watcher though, so if they change it won't be reflected.
  • This PR technically also adds support for _files and _solution because it made the code simpler. It might be something people want to use in the future though so this is probably a good thing?
  • 👉 This PR also refactored a bit the webcontainer-files.ts a bit with extra tests to make it easier to maintain this code.

Copy link

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

Copy link

changeset-bot bot commented Jun 26, 2024

⚠️ No Changeset found

Latest commit: ee46769

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.

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

Copy link

cloudflare-workers-and-pages bot commented Jun 26, 2024

Deploying tutorialkit-demo-page with  Cloudflare Pages  Cloudflare Pages

Latest commit: d7e184e
Status: ✅  Deploy successful!
Preview URL: https://fcb625ca.tutorialkit-demo-page.pages.dev
Branch Preview URL: https://joan-template-extends.tutorialkit-demo-page.pages.dev

View logs

@Nemikolh Nemikolh requested review from SamVerschueren and d3lm June 26, 2024 13:44
@Nemikolh Nemikolh marked this pull request as ready for review June 26, 2024 13:44
@Nemikolh Nemikolh requested a review from AriPerkkio June 26, 2024 13:44
Copy link

cloudflare-workers-and-pages bot commented Jun 26, 2024

Deploying tutorialkit-docs-page with  Cloudflare Pages  Cloudflare Pages

Latest commit: ee46769
Status: ✅  Deploy successful!
Preview URL: https://04a27249.tutorialkit-docs-page.pages.dev
Branch Preview URL: https://joan-template-extends.tutorialkit-docs-page.pages.dev

View logs

@@ -0,0 +1,4 @@
export const IGNORED_FILES = ['**/.DS_Store', '**/*.swp'];
export const EXTEND_CONFIG_FILEPATH = '/.tk-config.json';
Copy link
Member Author

Choose a reason for hiding this comment

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

Interested to hear feedback / thoughts on the name of the config file here. 👀

I started with template.json, then renamed it to .template.json, then .config.json to finally going with .tk-config.json.

My reasoning has been: I started with template.json because this is for templates. Then I thought, this might collide with user files, so added the starting .. I then realised that the code would also work for _files and _solution because we don't distinguish them and so thought maybe a more generic name would be better. I went with .config.json but figured that adding a tk- prefix would make it more explicitely TutorialKit specific and help tooling like the VSCode extension, to provide a schema for the JSON.

cc @sulco

}
}

export class FilesMap {
Copy link
Member Author

Choose a reason for hiding this comment

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

I created this class to make it easier to understand the graph structure and encapsulate the logic around:

  • Cycle detection (which should not throw or cause an infinite loop)
  • The conversion from a node in that graph to a Files entity that can be JSON serialized later

Comment on lines 213 to 215
if (path.sep !== '/') {
return result.replaceAll('\\', '/');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this should fix a bug on windows which I think the current TutorialKit has where files inside sub-folders will use \\ when they should use /.

Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

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

I've now tested this locally and it works really well, nice! 🙌

Here you can see how much code repetition I could reduce in a project that has 4 templates: AriPerkkio/tutorial-vite-plugin#6

src/templates
  ├── default
  │   ├── content.yaml
  │   ├── index.html
  │   ├── index.js
  │   ├── main.js
  │   ├── package-lock.json
  │   ├── package.json
  │   ├── public
  │   │   └── vite.svg
  │   ├── style.css
  │   ├── vite.config.internal.ts
  │   └── vite.config.ts
  │
  ├── empty  # Template that's used in terminal-only Node lesson
  │   └── index.js
  │
  ├── env-plugin
+ │   ├── .tk-config.json   # { "extends": "../default" }
- │   ├── index.html
  │   ├── index.js
- │   ├── main.js
- │   ├── package-lock.json
- │   ├── package.json
- │   ├── public
- │   │   └── vite.svg
  │   ├── style.css
- │   ├── vite.config.internal.ts
- │   └── vite.config.ts
  │
  └── replace-plugin
+     ├── .tk-config.json   # { "extends": "../default" }
-     ├── index.html
      ├── main.js
-     ├── package-lock.json
-     ├── package.json
-     ├── public
-     │   └── vite.svg
      ├── style.css
      ├── tutorial-example.js
-     ├── vite.config.internal.ts
-     └── vite.config.ts

@Nemikolh
Copy link
Member Author

Nemikolh commented Jun 27, 2024

@AriPerkkio That is indeed a lot!! 🤯 🤯

Your PR alone really shows how useful this feature is! Thanks for suggesting it 🙌

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to use this vite-app-2 in some lesson in packages/template/content.

@Nemikolh Nemikolh requested a review from AriPerkkio June 27, 2024 09:30
@Nemikolh Nemikolh merged commit c4350a8 into main Jun 27, 2024
8 checks passed
@Nemikolh Nemikolh deleted the joan/template-extends branch June 27, 2024 17:34
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