ChangeTheme: Sometimes leaves 2 or more links in the `<head>` #6476

bloodykheeng · 2024-04-26T09:21:20Z

Describe the bug

ive noted that everytime i call change theme it duplicates the link element am refereing to in the head section so which causes me issuses when am changing the theme though i had the manually first always delete exsting nodes by doing this
if (existingLinks.length > 1) { // Assuming that the first link is the old one and the new one is appended last, // we remove the first occurrence. document.head.removeChild(existingLinks[0]); } });

here is my whole code

import React, {
  createContext,
  useContext,
  useState,
  useEffect,
  useCallback
} from "react";
import { PrimeReactContext } from "primereact/api"; // Import the context
import PrimeReact from "primereact/api";

const ThemeContext = createContext();

export const ThemeProvider = ({ children }) => {
  const [theme, setTheme] = useState(
    () => localStorage.getItem("theme") || "light"
  );

  //   const [theme, setTheme] = useState("dark");
  const { changeTheme } = useContext(PrimeReactContext); // Access PrimeReact API through context

  const getThemeFromLocalStorage = () => {
    const linkId = "theme-link";
    console.log("current theme is : ", theme);
    changeTheme(`lara-light-blue`, `lara-${theme}-blue`, linkId, () => {
      const existingLinks = document.querySelectorAll(`link[id="${linkId}"]`);
      if (existingLinks.length > 1) {
        // Assuming that the first link is the old one and the new one is appended last,
        // we remove the first occurrence.
        document.head.removeChild(existingLinks[0]);
      }
    });
    return;
  };

  const changeMyTheme = () => {
    const newTheme = theme === "dark" ? "light" : "dark";
    const linkId = "theme-link";
    changeTheme(`lara-${theme}-blue`, `lara-${newTheme}-blue`, linkId, () => {
      setTheme(newTheme);
      localStorage.setItem("theme", newTheme);
      console.log("current theme in local storage : ", newTheme);
    });
    return;
  };

  let myTheme = theme;

  useEffect(() => {
    getThemeFromLocalStorage();
  }, []);

  const toggleTheme = () => {
    let currentTheme;
    if (theme === "light") {
      currentTheme = "dark";
    } else {
      currentTheme = "light";
    }

    // setTheme(currentTheme);
    // setTheme((current) => (current === "light" ? "dark" : "light"));
    changeMyTheme();
  };

  return (
    <ThemeContext.Provider value={{ theme, toggleTheme }}>
      {children}
    </ThemeContext.Provider>
  );
};

export const useTheme = () => useContext(ThemeContext);

here is the link element am refering two in the dom head section

  <link id="theme-link" rel="stylesheet" href="/themes/lara-light-blue/theme.css">


  <title>React App</title>
</head>

i dono if this is a bug or what but i hope this get solved.

Reproducer

No response

PrimeReact version

10.6.2

React version

17.x

Language

ES6

Build / Runtime

Create React App (CRA)

Browser(s)

chrome

Steps to reproduce the behavior

No response

Expected behavior

I expect the change theme to just reference to the link element in the head and it changes the path not creating new link element every time i call it in a different function

The text was updated successfully, but these errors were encountered:

sja-cslab · 2024-04-26T11:15:51Z

I don't really understand what exactly you're doing there. However, I don't think that this is something PrimeReact could fix. Why are you doing things so complicated?

Just do something like:

<link id="theme" rel="stylesheet" type="text/css" href="/my/default/theme.css">

let themeLink = document.getElementById('theme');

if (themeLink) {
    themeLink.href = `/themes/${themeId}/theme.css`;
}

bloodykheeng · 2024-04-26T11:47:51Z

am changing between dark and light theme following prime react docs your solution was what i tried first but the problem with it during transition or the process of changing the theme you can see the elements unstyled for like 2 seconds before a theme gets updated which makes it wrong approach i think

melloware · 2024-04-26T12:24:21Z

@bloodykheeng so you are saying it has 2 <link> in the head and you are manually removing one. Are you saying your code posted above fixes the problem and the docs need updating OR you still have the issue? a reproducer will help.

sja-cslab · 2024-04-26T12:51:24Z

@melloware i can reproduce it on the doc.

However, it took a while to switch themes over and over and it felt like i was too fast one time which causes this

melloware · 2024-04-26T12:53:03Z

ahhh it might be a timing thing with the hooks and order its firing...

sja-cslab · 2024-04-26T12:53:43Z

bloodykheeng may work in strict mode which causes double render then?

Edit:
changeTheme as useCallback could do the trick!?

sja-cslab · 2024-04-26T13:00:10Z

@bloodykheeng you're right - if you dont have any fallback theme, the switch can cause a moment of unstyle thing because the css is loaded over the network

melloware · 2024-04-26T13:02:12Z

@sja-cslab let me know if we need to fix the code or update the docs or both.

bloodykheeng · 2024-04-26T13:12:58Z

@melloware @sja-cslab What i noted is that every time i call this changeTheme it creates a link element in the head of the document if u have only one function that calls changeTheme every thing works fine but if u call it in another place that will invoke two links in that head thats why in my code i had to put a call back that manually removes one link from the head
const getThemeFromLocalStorage = () => { const linkId = "theme-link"; console.log("current theme is : ", theme); changeTheme(lara-light-blue, lara-${theme}-blue, linkId, () => { const existingLinks = document.querySelectorAll(link[id="${linkId}"]); if (existingLinks.length > 1) { // Assuming that the first link is the old one and the new one is appended last, // we remove the first occurrence. document.head.removeChild(existingLinks[0]); } }); return; };

see the call back in the above function i use that function to get the theme i stored in local storage on initial mount so it gets it and sets it fine but now after that i have this other one too

const changeMyTheme = () => { const newTheme = theme === "dark" ? "light" : "dark"; const linkId = "theme-link"; changeTheme(lara-${theme}-blue, lara-${newTheme}-blue, linkId, () => { setTheme(newTheme); localStorage.setItem("theme", newTheme); console.log("current theme in local storage : ", newTheme); }); return; };

the above changes the theme everytime user clicks on a button in the app so i notted that using two changeTheme hooks in my context creates two links of the same id in the head which causes an accident when the app css styles are running it ends up that this changeMyTheme() will never work correctly because it will be changing the first link so that why i said i had to manually remove a one link from the head section on intial load after applying the theme from users local storage by doing this if (existingLinks.length > 1) { // Assuming that the first link is the old one and the new one is appended last, // we remove the first occurrence. document.head.removeChild(existingLinks[0]); }

here is my entire code i had placed in the context

`import React, {
createContext,
useContext,
useState,
useEffect,
useCallback
} from "react";
import { PrimeReactContext } from "primereact/api"; // Import the context
import PrimeReact from "primereact/api";

const ThemeContext = createContext();

export const ThemeProvider = ({ children }) => {
const [theme, setTheme] = useState(
() => localStorage.getItem("theme") || "light"
);

// const [theme, setTheme] = useState("dark");
const { changeTheme } = useContext(PrimeReactContext); // Access PrimeReact API through context

let myTheme = theme;

useEffect(() => {
getThemeFromLocalStorage();
}, []);

const toggleTheme = () => {
let currentTheme;
if (theme === "light") {
currentTheme = "dark";
} else {
currentTheme = "light";
}

// setTheme(currentTheme);
// setTheme((current) => (current === "light" ? "dark" : "light"));
changeMyTheme();

};

return (
<ThemeContext.Provider value={{ theme, toggleTheme }}>
{children}
</ThemeContext.Provider>
);
};

export const useTheme = () => useContext(ThemeContext);
`

sja-cslab · 2024-04-26T13:14:21Z

@melloware, the docs are just fine - I would say it's a bug.

And yeah, it seems to be a timing problem. The following happens:

1: Find the link element.
2: Clone it.
3: Set the ID of the clone to oldId-clone and set the href to the new URL.
4:Add a "load" event listener (async).
5: Insert the clone before the original link.
6: Loading is done - remove the old element and remove "-clone" from the clone's ID.

The issue arises with steps 5 and 6. If a second clone is created and inserted right before the loaded event triggers, we end up with two clones, both of which are renamed to the original ID.

Question here is, how would you like to prevent that? We could check for an already available clone e.g.

// Check if a cloned link element already exists
    const existingCloneElement = document.getElementById(linkElementId + '-clone');

     // If a cloned link element already exists, remove it from the document
     existingCloneElement?.remove();

bloodykheeng · 2024-04-26T13:19:18Z

@sja-cslab well i think u should make sure after updating the link path then either delete the original or the clone such that we don't end up with two links of same id pointing to same css styles

sja-cslab · 2024-04-26T13:20:27Z

@bloodykheeng if you delete the original element before the load, you'll end up with the effect, that you got a timespan without styling

bloodykheeng · 2024-04-26T13:24:58Z

@sja-cslab see how i did it by deleting the parent in the frrom the changeTheme call back
const existingLinks = document.querySelectorAll(link[id="${linkId}"]); if (existingLinks.length > 1) { // Assuming that the first link is the old one and the new one is appended last, // we remove the first occurrence. document.head.removeChild(existingLinks[0]);

then is the whole

const getThemeFromLocalStorage = () => { const linkId = "theme-link"; console.log("current theme is : ", theme); changeTheme(lara-light-blue, lara-${theme}-blue, linkId, () => { const existingLinks = document.querySelectorAll(link[id="${linkId}"]); if (existingLinks.length > 1) { // Assuming that the first link is the old one and the new one is appended last, // we remove the first occurrence. document.head.removeChild(existingLinks[0]); } }); return; };

sja-cslab · 2024-04-26T13:35:20Z

I see what you're doing there, but you can't be sure that this would always work. As you say in the comment, "Assuming," which should not be used here. Because in different timing/callchain scenarios, this could still cause the wrong theme to load. So it's better to avoid that as soon as possible.

In your example, let's say you load theme A, then B, but B loads faster (because, for example, it's smaller), so you would delete B even if the user swapped to it last

PS: I'm not sure if that example is correct but I think it explains what could happen

bloodykheeng · 2024-04-26T15:44:38Z

@sja-cslab ur right i think what prime react team members should do is to change the change theme logic in that were as it clones let it alwas delete the first link that is at index 0 if it exists or else lets change this strategy of manipulating the dom and just apply a layer in some config.js like how tailwind does

bloodykheeng added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Apr 26, 2024

melloware added Status: Needs Reproducer Issue needs a runnable reproducer and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Apr 26, 2024

This comment was marked as off-topic.

Sign in to view

melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Reproducer Issue needs a runnable reproducer labels Apr 26, 2024

melloware changed the title ~~Change Theme From prime react creates two duplicate links in the head~~ Change Theme: Sometimes leaves 2 or more links in the <head> Apr 26, 2024

melloware changed the title ~~Change Theme: Sometimes leaves 2 or more links in the <head>~~ ChangeTheme: Sometimes leaves 2 or more links in the <head> Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ChangeTheme: Sometimes leaves 2 or more links in the `<head>` #6476

ChangeTheme: Sometimes leaves 2 or more links in the `<head>` #6476

bloodykheeng commented Apr 26, 2024 •

edited by melloware

sja-cslab commented Apr 26, 2024 •

edited

bloodykheeng commented Apr 26, 2024

This comment was marked as off-topic.

melloware commented Apr 26, 2024

sja-cslab commented Apr 26, 2024

melloware commented Apr 26, 2024

sja-cslab commented Apr 26, 2024 •

edited

sja-cslab commented Apr 26, 2024

melloware commented Apr 26, 2024

bloodykheeng commented Apr 26, 2024 •

edited

sja-cslab commented Apr 26, 2024 •

edited

bloodykheeng commented Apr 26, 2024

sja-cslab commented Apr 26, 2024

bloodykheeng commented Apr 26, 2024

sja-cslab commented Apr 26, 2024 •

edited

bloodykheeng commented Apr 26, 2024

ChangeTheme: Sometimes leaves 2 or more links in the <head> #6476

ChangeTheme: Sometimes leaves 2 or more links in the <head> #6476

Comments

bloodykheeng commented Apr 26, 2024 • edited by melloware

Describe the bug

Reproducer

PrimeReact version

React version

Language

Build / Runtime

Browser(s)

Steps to reproduce the behavior

Expected behavior

sja-cslab commented Apr 26, 2024 • edited

bloodykheeng commented Apr 26, 2024

This comment was marked as off-topic.

melloware commented Apr 26, 2024

sja-cslab commented Apr 26, 2024

melloware commented Apr 26, 2024

sja-cslab commented Apr 26, 2024 • edited

sja-cslab commented Apr 26, 2024

melloware commented Apr 26, 2024

bloodykheeng commented Apr 26, 2024 • edited

sja-cslab commented Apr 26, 2024 • edited

bloodykheeng commented Apr 26, 2024

sja-cslab commented Apr 26, 2024

bloodykheeng commented Apr 26, 2024

sja-cslab commented Apr 26, 2024 • edited

bloodykheeng commented Apr 26, 2024

ChangeTheme: Sometimes leaves 2 or more links in the `<head>` #6476

ChangeTheme: Sometimes leaves 2 or more links in the `<head>` #6476

bloodykheeng commented Apr 26, 2024 •

edited by melloware

sja-cslab commented Apr 26, 2024 •

edited

sja-cslab commented Apr 26, 2024 •

edited

bloodykheeng commented Apr 26, 2024 •

edited

sja-cslab commented Apr 26, 2024 •

edited

sja-cslab commented Apr 26, 2024 •

edited