Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fast Refresh with Components Using Components #3

Closed
ArielOfAtlantica opened this issue Sep 5, 2024 · 3 comments
Closed

Fast Refresh with Components Using Components #3

ArielOfAtlantica opened this issue Sep 5, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@ArielOfAtlantica
Copy link

What version of Temple are you using?

0.1.4

What OS are you experiencing this issue?

Mac

What Browser are you experiencing this issue?

Google Chrome Latest

Describe the Bug

So I have an import structure like the following:

index.dtml
  - footer.tml
    - manifest.tml
  • When I update index.dtml, the page reloads (That's fine for now).
  • When I update footer.tml, fast refresh also is working.
  • When I update manifest.tml This is where fast refresh doesn't update the DOM correctly.

If I refresh the page manually the DOM becomes correct again.

Expected Behavior

If I update a compoent that is imported by another component that is imported by a document, I expect that the DOM updates correctly.

To Reproduce

Here are all my templates.

page.dtml

<link rel="import" type="component" href="./footer.tml" />
<style>
  @tailwind base;
  @tailwind components;
  @tailwind utilities;
</style>
<script>
  import { env, props } from '@ossph/temple';
  const { BUILD_ID, APP_DATA, NODE_ENV } = env();
  const { title } = props();
</script>
<html>
  <head>
    <title>{title}</title>
    <link rel="stylesheet" type="text/css" href={`/build/${BUILD_ID}.css`} />
    <script data-app={APP_DATA} src={`/build/${BUILD_ID}.js`}></script>
    <if true={NODE_ENV !== 'production'}>
      <script src="/dev.js"></script>
    </if>
  </head>
  <body>
    <main class="py-24">
      <h1 class="text-center">{title}</h1>
    </main>
    <footer year={2021} href="https://www.acmeinc.com">
      {'Acme Inc'}
    </footer>
  </body>
</html>

footer.tml

<link rel="import" type="component" href="./manifest.tml" />
<script>
  import { props, children } from '@ossph/temple';
  const { year, href } = props();
  const company = children();
</script>
<footer class="text-center">
  <manifest {year} {href} {company} />
</footer>

manifest.tml

<script>
  import { props } from '@ossph/temple';
  const { year, href, company } = props();
</script>
&copy; {year} <a href={href}>{company}</a>
  1. Make a change in footer.tml
  2. Then make a change in manifest.tml

Include git repo/fork so we can easily reproduce the issue

No response
@ArielOfAtlantica ArielOfAtlantica added the bug Something isn't working label Sep 5, 2024
@cblanquera
Copy link
Collaborator

Thanks for reporting this! @ossph/temple-dev is still a WIP so any issues reported is always welcome.

@cblanquera
Copy link
Collaborator

I see you are passing the children of <footer> to <manifest> as a prop, then have manifest send it back out as children of the <a>. The problem with this way is that children is an array of Elements presumably already in the DOM (or already appended somewhere). Whenever a component re-renders it produces a new set of elements, making the previous elements orphans. This is why the rest of the props are working while company is not showing, because it's was appended to your <a> element but appended again in an orphan <a>. This is more of the DOM's doing than Temple.

You can fix this in your manifest.tml by getting the textContent of the children instead like the following.

<script>
  import { props } from '@ossph/temple';
  const { year, href, company } = props();
  const name = company[0].textContent;
</script>
&copy; {year} <a href={href}>{name}</a>

Then resave footer (as is) or reload the page. When you make changes now it should fast refresh properly.

@ArielOfAtlantica
Copy link
Author

Kind of makes sense. Definitely agree that putting children back into the props is kind of weird. I was trying to see if it works :) your work around is acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cblanquera @ArielOfAtlantica