-
Notifications
You must be signed in to change notification settings - Fork 5
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
In .args.mjs
allow people to pass JS variables instead of strings
#116
Comments
Could you explain this? It's already allowed, you just need to add logic to template to handle that vars |
@rin-st I think it's related to this: #109 (comment) |
I tried to experiment with this and created draft pr #118 . Main idea is stringifying passed variables if it's not a string #118 (comment) But solution has a (big) cons. Let's take a look at these extension variables: https://github.com/carletex/create-eth-extensions/blob/tailwind-extend-solidity-version/extension/packages/nextjs/tailwind.config.js.args.mjs We need to be able to pass an object and string representation of const lightTheme = {
something: "#000"
...
} and it's string representation like it worked before const lightTheme = `{
something: "#000"
...
}` which used this way
[Issue 1] const extendTheme = {
backgroundImage: {
"gradient-radial": "radial-gradient(var(--tw-gradient-stops))",
},
colors: {
"custom-blue": "#1E40AF",
}
}; and it's string representation. Note that there's no "{" and "}" at the start and at the end const extendTheme = `
backgroundImage: {
"gradient-radial": "radial-gradient(var(--tw-gradient-stops))",
},
colors: {
"custom-blue": "#1E40AF",
}
`; which is used here:
we meet first issue: we can't just pass an object here, so we need additional logic to remove that "{" and "}" if object is passed https://github.com/scaffold-eth/create-eth/pull/118/files#r1757585413 [issue 2] export const menuObjects = `{
label: "Greetings",
href: "/greetings",
icon: <MagnifyingGlassIcon className="h-4 w-4" />,
}`; if we want to pass an object, we can't pass it just like that, sending JSX here is a bad idea. export const menuObjects = {
label: "Greetings",
href: "/greetings",
icon: <MagnifyingGlassIcon className="h-4 w-4" />,
}; So we need to pass JSX as a string export const menuObjects = {
label: "Greetings",
href: "/greetings",
icon: '<MagnifyingGlassIcon className="h-4 w-4" />',
}; and again, we need additional dances to convert it to a valid result string, which is the same as first block on this [issue 2] https://github.com/scaffold-eth/create-eth/pull/118/files#r1757596314 . Works same way for any variables instead of JSX [issue 3] [issue 4] Probably there is a better way to pass an objects, but I didn't found it for now |
Also, I'm thinking about aggregating strategies for every variable, because we may need different behaviors #109 (comment). Possible strategies: collect, single, merge. we can add this strategies config to Line 1 in 4bec81e
cc @technophile-04 to see this thread |
Thanks @rin-st for nice research and tinkering and also other for bringing up this discussion! After thinking and doing research for a while, I feel like we don't need to change anything at We tell people to pass objects where syntax wise its intuitive(tailwind conf, menuObjects, etc..) and strings for example passing code snippets (example in And we handle the logic at `tailwind.config.js.template.mjs` :const contents = ({ lightTheme, darkTheme, extendTheme }) => {
const massagedExtendedTheme = JSON.stringify(extendTheme[0]);
return `
{
...
theme: {
extend: {
${extendTheme[0] && `...${massagedExtendedTheme},`}
boxShadow: {
center: "0 0 12px -2px rgb(0 0 0 / 0.05)",
},
animation: {
"pulse-fast": "pulse 1s cubic-bezier(0.4, 0, 0.6, 1) infinite",
},
},
},
};`;
}; For example in issue 2, I think its fine to allow people to pass icon as jsx string? And then in Header.template.mjs :const exampleMenuObjects = [
{
label: "EIP-712",
href: "/eip-712",
icon: `<DocumentCheckIcon className="h-4 w-4" />`,
},
{
label: "EIP-712",
href: "/eip-712",
icon: `<DocumentCheckIcon className="h-4 w-4" />`,
},
];
const contents = ({ menuIconImports, menuObjects }) => {
const massageMenuObjects = exampleMenuObjects.map(obj => {
return `{
label: "${obj.label}",
href: "${obj.href}",
icon: ${obj.icon},
}`;
});
console.log(massageMenuObjects);
return `
export const menuLinks: HeaderMenuLink[] = [
{
label: "Home",
href: "/",
},
{
label: "Debug Contracts",
href: "/debug",
icon: <BugAntIcon className="h-4 w-4" />,
},
${massageMenuObjects.join(",\n")},
${menuObjects.filter(Boolean).join(",\n ")}
];
`
} Regarding issue 3 I think it can be done now too? What if you do like this: hardhat.config.ts.args.mjs :export const networks = `
myFakeNetwork: {
url: \`https://my-fake-network.alchemyapi.io/v2/\${providerApiKey}\`,
accounts: ["0xmyPK"],
},
myFakeNetwork2: {
url: \`https://my-fake-network2.alchemyapi.io/v2/\${providerApiKey}\`,
accounts: ["0xmyPK"],
}`; or I missed the point in issue3 probably you were mentioning something different? Issue 4 again could be solved if scope our massaging in that All in all maybe feels like doing lot of work in also I think regarding choosing when should allow people string and when object maybe it depends on that particular file and how ergonomic it is for people to copy past that code from instance to their Lol if I think maybe our current |
How, using comments? For example if by default object is empty. Because if string will be passed everything will break.
Yes, but we need to pass JSX or any variables as strings anyway
That private keys will be in result hardhat.config.js and will leak. We need to pass it as string again To sum up, as I understand you mean the same as I wrote here #116 (comment) . We can just not change anything, just take object and handle it inside template. But if using objects we need to always pass variables as strings. So passing objects doesn't add so much value. |
#118 adds possibility to use both strings and objects, but that variables in objects need to be stringified anyway, so regarding objects it's the same. But it adds complexity, because we're handling both cases. If we find a way to tell users what we need to use, object or string, then agree, we don't need that pr at all. And yes, we can handle both string and object in every template without stringifying in |
Yess lol! and #109 (comment) works out box
The way we are doing it currently is through TEMPLATE_FILES.md tell them to look up at correponding |
So, closing issue and pr?
Is it right? |
Closing this thanks all for the discussion! |
No description provided.
The text was updated successfully, but these errors were encountered: