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

Ability to set GOOBER_ID #535

Open
ecustic opened this issue Feb 4, 2023 · 5 comments
Open

Ability to set GOOBER_ID #535

ecustic opened this issue Feb 4, 2023 · 5 comments
Labels
discussion feature New feature or request

Comments

@ecustic
Copy link

ecustic commented Feb 4, 2023

Is there any chance of getting a way to change the GOOBER_ID used, so that we don't end up with window._goober?

My issue is that I'm developing a plugin for Obsidian and would like to use goober for my styles. The convention here is for each plugin to clean up any thing it has added to the dom when it unloads. In this case this means removing the #_goober element. The issue is if any other plugin happens to also want to use goober, I'm taking their styles with me when my plugin unloads.

Even setting the target doesn't actually work here, since it still creates a style tag with #_goober inside it meaning it still polutes window._goober.

I've seen proposals for creating multiple instances of goober, and that would be the optimal solution. But if there is a quicker way to just expose the GOOBER_ID as a configurable value for an entire project that would also be extremely helpful.
@ecustic
Copy link
Author

ecustic commented Feb 4, 2023 •
edited
Loading

Alternatively just allowing us to point the target directly to a style tag would also solve the issue for me.

goober/src/core/get-sheet.js

Lines 11 to 24 in 2b2d422

export let getSheet = (target) => {
if (typeof window === 'object') {
// Querying the existing target for a previously defined <style> tag
// We're doing a querySelector because the <head> element doesn't implemented the getElementById api
return (
(target ? target.querySelector('#' + GOOBER_ID) : window[GOOBER_ID]) ||
Object.assign((target || document.head).appendChild(document.createElement('style')), {
innerHTML: ' ',
id: GOOBER_ID
})
).firstChild;
}
return target || ssr;

Maybe adding an early return to the getSheet function here:

 export let getSheet = (target) => { 
     if (typeof window === 'object') { 
        if (target instanceof HTMLStyleElement) {
            return target;
        }
     //...
     }
     //...
}

@cristianbote cristianbote added feature New feature or request discussion labels Feb 6, 2023
@cristianbote
Copy link
Owner

Hey @ecustic, thanks for opening this issue! Nice topic 😃

Before getting into it just wanted to let you know that I don't have knowledge on the Obsidian inner workings.

About polluting the window, would goober the only one that would be polluting? More precisely I believe most of the UI related frameworks will pollute in a way or another.
Creating goober instances is another way of handling these as well and I've been thinking about it. One of the things that I don't really like is that with instances the import of goober would have to be changed a bit:

// src/utils/styled.js
export const styled = createGoober({
 // here will go the config part
});

// src/components/button.js
import { styled } from 'src/utils/styled';

Now, the question is, is this better? Debatable 😅

@ecustic
Copy link
Author

ecustic commented Feb 6, 2023 •
edited
Loading

I think the optimal solution would be to have both.

Export { styled, css, createGoober }. createGoober should return { styled, css }. The module should then internally call createGoober with default config and expose the styled and css it returns. This would allow the API to remain unchanged for the 99% who use goober as it currently is, while still having a single implementation instead of differentiating between instanced and global implementations.

@ecustic
Copy link
Author

ecustic commented Feb 6, 2023

About polluting the window, would goober the only one that would be polluting? More precisely I believe most of the UI related frameworks will pollute in a way or another.

This is very true and one of the reasons I chose goober after being frustrated with the tools I normally use: emotion and styled-components. There are some unique limitations when you're developing an app which is going to be plugged-in to a larger codebase and expected to co-exist without any knowledge of what anything else is doing. Out of all the CSS-in-JS frameworks I tested, goober ended up being the easiest to wrangle into doing what I needed. Specifically due to it being bound to a single identifier, which I could find and remove when needed.

To avoid clashes I'm currently using an extremely hacky temporary solution where I've just written a small esbuild plugin to replace the three instances of the string _goober in the resolved goober module to _my_plugin_name_goober.

But in fact there should be no need to use id at all. Replacing the id with a data attribute such as data-goober-id and then having the default value just be 'goober', but let users configure the id, would work just as well but not polute window.

@wateryoma
Copy link

Hello,
since setup call is mandatory - we can optionally pass id to setup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discussion feature New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ecustic @cristianbote @wateryoma