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.

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

Trial on Ecosystem v0.2 #111

Closed
39 of 44 tasks
rufuspollock opened this issue Aug 23, 2022 · 25 comments
Closed
39 of 44 tasks

Trial on Ecosystem v0.2 #111

rufuspollock opened this issue Aug 23, 2022 · 25 comments
Assignees
Labels
enhancement New feature or request

Comments

@rufuspollock
Copy link
Member

rufuspollock commented Aug 23, 2022

This is also a chance to trial out a sort of upgrade.

Acceptance

Key back-ported features ✅ 2022-09-14

  • Some way to configure title of site in navbar e.g. navbarTitle attribute with logo, text, version options. ✅ FIXED set values with navbarTitle attribute in user's config
  • If search settings not configured don't show search bar ** ✅ FIXED based on algolia config values**
  • Ability to only have one theme (e.g. not light and dark) => hide switcher, only have that theme ✅ FIXED
  • Misc small fies

Learnings

Upgrading Flowershow on a site that uses contentlayer

To address:

  • components not importing in markdown - Big setback to resolve
  • document types are modified in contentlayer
  • package.json is replaced completely

Tasks

  • Create script for upgrading ✅2022-08-29 created an upgrade.sh in packages/flowershow
  • Evaluate current setup of ecoystem to check for conflicts and hand merging 🚧2022-08-29 see notes and tasks below

Fixes to flowershow round 1

  • 🐛 site won't build without nextSeo being defined but it is not in default config setup
    components/MDX.js (39:14) @ MdxPage
  37 |           }
  38 |         ] 
> 39 |       : siteConfig.nextSeo.openGraph.images,
     |        ^
  40 |   }}
  41 | />
  42 | <Component
  • data/exampleData.js - why do we have this? Can this live somewhere else e.g. in site? olayway: exampleData moved to /site/data/exampleData; it makes import paths in MDX a bit ugly but I've created an issue for it - Simplify imports of custom components and data #154
  • siteConfig.js remove commented out nextSeo stuff (since commented out so just noise)
  • MDX.js ✅ 2022-08-31
    • Backport Link
    • Backport svg: props => <Fragment {...props} />
  • Nav.js We have hard-coded github icon but that should be optional ✅ 2022-08-31
  • compontents/TempCallout.js olayway: this one is a temporary substitution for callouts that we're not supporting yet, I use it in docs to wrap some important info in a nice border; it will be removed once we support callouts
  • why do we have a maxWidth mod to tailwindconfig.js - good to have a comment in code for this 94e16ce#r82529574 ✅ 2022-08-31
  • Why does flowershow have testData array in it? olayway: I was just testing sth and wanted to add a section in our docs about how to create global components or data. There is also GlobalTest component in components object in MDX file - I've just removed both of them and created an issue to remember to add this info about global props/components to our docs Make (site) config and page frontmatter available as variables in a given page #155

siteConfig.js

MDX.js

  • Refactor mdx files to import the components they need
    • Search
    • TernaryPlot
    • CircularVis
    • Hero
  • Refactor components to load data themselves where possible ✅ 2022-09-06
  • Search from flowershow and ecosystem Search component conflict ✅ 2022-09-06
    • rename ecosystem search component to be called "ProfileSearch"
    • fixes imports

Fixes in ecosystem Round 2

  • move ecosystem images to assets content/assets/images (skip profile images for now) ❌2022-09-08 not required IMO - just symlink the public stuff you need into public ✅ 2022-09-12
  • fix logo title and author logo ✅ 2022-09-12
  • fix footer 'made with flowershow' ❌2022-09-08 don't fix as will be fixed once we do 3a9cd9f ✅ 2022-09-12
  • force light theme **✅2022-09-08 **

Remaining in ecosystem Round 3

  • Commit any definite local fixes in ecosystem ✅ 2022-09-08
  • Backport remaining fixes ✅ 2022-09-08 [#111]: Backport changes from ecosystem #161
    • configs for navbar title, theme
    • forced single theme option (no theme toggle icon)
    • render searchbar if algolia configs are present
    • public/_flowershow folder for flowershow assets (logo and theme button)
  • Do final upgrade ✅ 2022-09-08 on flowershow-v0.2
  • Merge that (tedious!)
  • Write notes
  • Done!

pages/[slug...].js

  • We need to pass orgs as a prop as well as page ⏭️ could we avoid this by refactoring components like search to load the org profiles themselves
    • 🚩 how do we deal with passing additional data down for e.g. specific types like Profile page ⏭️ at least for ecosystem we assume that we don't need to since e.g. page has the data it needs

Results round 2

Results round 1

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   components/Layout.js  ✅
	modified:   components/MDX.js     🚧 back port some stuff
	modified:   components/Nav.js     🚧 github issues
	modified:   components/Search.js  🚩 conflict between local and flowershow. Need to rename local.
	modified:   config/siteConfig.js  🚧
	modified:   contentlayer.config.js🚧
	modified:   layouts/docs.js       ✅
	modified:   layouts/unstyled.js   ✅
	modified:   package-lock.json     🚧
	modified:   package.json          🚧
	modified:   pages/[[...slug]].js  🚧 - orgs prop
	modified:   pages/_app.js         ✅
	modified:   tailwind.config.js    🚧 MERGE

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	.env.example                   ❌
	components/MobileNavigation.js ✅
	components/Pre.js              ✅
	components/TempCallout.jsx     ❌
	data/                          ❌
	layouts/blog.js                ✅
	netlify.toml                   ❌
	public/assets                  ✅
	sandbox/                       ❌
	styles/                        ✅
rufuspollock added a commit that referenced this issue Aug 29, 2022
… script from experience upgrading ecosystem in #111.
@rufuspollock
Copy link
Member Author

Have started to make progress on this.

@rufuspollock
Copy link
Member Author

Created an upgrade script and documented in https://flowershow.app/docs/upgrade

@khalilcodes
Copy link
Contributor

khalilcodes commented Sep 5, 2022

@rufuspollock

Current steps (for MDX.js in tasks) on upgrading a nextjs site that already uses contentlayer

before doing sh upgrade.sh

  • Renamed Search to ProfileSearch.js
    • had to create an empty Search.js file since after running upgrade.sh it still modified and conflicted with the ProfileSearch.js component due to commits.

after doing sh upgrade.sh

  • had to manually add:
    • in contentlayer.config.js ecosystem document types (Topics, etc)
    • in package.json ecosystem dependencies (fuse.js, d3, d3-ternary, @tailwindcss/forms)

Screen Shot 2022-09-05 at 2 07 50 PM

@rufuspollock
Copy link
Member Author

had to create an empty Search.js file since after running upgrade.sh it still modified and conflicted with the ProfileSearch.js component due to commits.

I don't get this quite. can you explain more.

@khalilcodes
Copy link
Contributor

khalilcodes commented Sep 5, 2022

After renaming Search.js in ecosystem to ProfileSearch.js, I ran sh upgrade.sh. Instead of it creating a new Search.js component/file which should come from flowershow, it added the search code to ProfileSearch.js and renamed it back to Search.js. This is probably because I did not commit the changes and ran the upgrade maybe. So I just created a new file called Search.js and then ran the upgrade.

@rufuspollock
Copy link
Member Author

@khalilcodes this seems really weird to me. How would it create a ProfileSearch.js file when there is none in templates/default afaict (there is a Search.js).

@khalilcodes
Copy link
Contributor

khalilcodes commented Sep 6, 2022

@rufuspollock pushed

  • refactor components to load data themselves
  • rename ecosystem search / add flowershow search

changes to flowershow for ecosystem:

  • MDX.js SEO image url
    • in flowershow was frontMatter.image
    • in ecosystem is flowershow.image.url (image field here is nested and has url, cached, cached_new)

could not resolve:

  • importing components in mdx

Screen Shot 2022-09-06 at 3 02 37 AM

TODO:

  • add images to assets content/assets/images
  • resolve assets the symlinked folder in public. Showing no such file or directory ?

Screen Shot 2022-09-06 at 12 04 37 PM

  • in contentlayer.config.js backport renaming field to url_path or _url ?
  • fix logo title and author logo (in flowershow ?)
  • add dark theme styles in ecosystem components

@rufuspollock
Copy link
Member Author

@khalilcodes very good summary. can you move the key stuff up into the description and also chat with @olayway especially about the mdx import stuff that does not work.

@olayway
Copy link
Member

olayway commented Sep 7, 2022

Tbh, I'm not really sure why we are testing it all this way 😅 (but probably it's because I haven't been working on it with you @khalilcodes and @rufuspollock, so please take it all with a grain of salt).

So, it looks to me that we're trying to convert a site that was not really built with Flowershow into a Flowershow one, and it seems to me it's doomed to have a lot of issues every time we do this (probably less and less, but still..).

My question is (I know it may be a silly one): Why don't we try creating Ecosystem site starting from the fresh Flowershow template and slowly adding everything from the original Ecosystem site (just as if some user would like to create if from scratch with Flowershow)? And actually having Flowershow template (e.g. inside .flowershow) and Ecosystem specific (i.e. custom, user-defined) stuff separate? And solve any errors that we encounter ONE-BY-ONE with this separation in mind, i.e. thinking "how can we improve Flowershow's template API (configuration options etc.) so that the user doesn't have to touch .flowershow to do X". I mean, in our case Ecosystem site is kind of a user, isn't it?
To me, it's just kind of a messy approach and even if we make it work, to me it's almost impossible to say what are the lessons learned or why we had to make a specific change to Flowershow (an example would be our last svg "backport" issue).

Another example:

changes to flowershow for ecosystem:

  • MDX.js SEO image URL
    • in flowershow was frontMatter.image
    • in ecosystem is flowershow.image.url (image field here is nested and has URL, cached, cached_new)

I think, in order to learn something by doing this "upgrade" and actually improving Flowershow, instead of adjusting Ecosystem's MDX.js file (which is the file we don't want users to ever touch) just to make it work, I'd rather think about: can we/do we want to support this kind of nested fields as well? Can we somehow allow the user to configure it?
Also, this is just a quick thought, but it seems to me that if we take advantage of contentlayer's computed fields, we could flatten the nested fields and don't ever touch MDX. (The next question here of course is: can we import custom document type definitions in contentlayer.config.js).

@rufuspollock
Copy link
Member Author

@olayway

My question is (I know it may be a silly one): Why don't we try creating Ecosystem site starting from the fresh Flowershow template and slowly adding everything from the original Ecosystem site (just as if some user would like to create if from scratch with Flowershow)? And actually having Flowershow template (e.g. inside .flowershow) and Ecosystem specific (i.e. custom, user-defined) stuff separate? And solve any errors that we encounter ONE-BY-ONE with this separation in mind, i.e. thinking "how can we improve Flowershow's template API (configuration options etc.) so that the user doesn't have to touch .flowershow to do X". I mean, in our case Ecosystem site is kind of a user, isn't it?

It was kind of built with it - we did a big refactor in july to use flowershow v0.1 so this is an upgrade. the issue then is we didn't have mdx components working.

Re moving stuff into .flowershow: it doesn't really change anything here. The real issue is that ecosystem is a full nextjs site, not a pure one - i.e. i believe it has customized contentlayer. that said it is a good experience IMO to try this.

I think, in order to learn something by doing this "upgrade" and actually improving Flowershow, instead of adjusting Ecosystem's MDX.js file (which is the file we don't want users to ever touch) just to make it work, I'd rather think about: can we/do we want to support this kind of nested fields as well? Can we somehow allow the user to configure it?

Also, this is just a quick thought, but it seems to me that if we take advantage of contentlayer's computed fields, we could flatten the nested fields and don't ever touch MDX. (The next question here of course is: can we import custom document type definitions in contentlayer.config.js).

👍 i said this to @khalilcodes on the call the other day. This is exactly right approach IMO on this point.

@rufuspollock
Copy link
Member Author

@khalilcodes

add images to assets content/assets/images

Why do we need to do this? can't we just symlink the relevant folder over into public for now?

@khalilcodes
Copy link
Contributor

thanks @olayway for the insight and yes you are right that me and @rufuspollock have more context here since we have been working on the ecosystem from scratch.

My approach here is actually more user centric, and I believe my goal here is to find the bugs (and blockages that user would face while installing flowershow), rather than trying to make things just work. And since I have more context here, I guess I am able to figure out more clearly the approach whether new or existing. I'm sure once thing's look good to go here our next step would be to try it on a completely new one which would be just the obsidian folder. I suggest let's just look at this as a testing ground for a project that includes a lot of custom things.

On that note, what I found yesterday while working on the dark/light theme configuration is that I have not correctly implemented it in flowershow. For eg. what if the user just wants to change the theme and not the icon. Configuring this would result in bugs for the the theme toggle not rendering at all if I did as below.

const config = {
  // if I do this in config.js then siteConfig.js is overriden and 
  // it wont have the themeToggle icon svg in the config
  theme: {
    default: 'light'
  }
}

A better way to handle this would to extract them out separately and not keep it in an object.

// in siteConfig.js
const config = {
  defaultTheme: 'light'.
  themeToggleIcon: '...'
}

Also, what if a user doesn't want a theme toggle at all (as in ecosystem site).
I have made changes accordingly to ecosystem to suit the user needs better and would be backporting this change in flowershow, starting out with documentation first in a PR.

@rufuspollock

add images to assets content/assets/images
Why do we need to do this? can't we just symlink the relevant folder over into public for now?

As mentioned above, I was going more towards how a user would have it, but for now if that's not our need and we have the required info on it, I can just symlink the relevant folder.

@rufuspollock
Copy link
Member Author

Remaining:

  • Commit any definite local fixes in ecosystem
  • Backport remaining fixes
  • Do final upgrade
  • Merge that (tedious!)
  • Write notes
  • Done!

@olayway
Copy link
Member

olayway commented Sep 8, 2022

@khalilcodes, @rufuspollock it seems importing components that depend on external packages doesn't work indeed 😞 I believe it has to do with Contentlayer's configuration/usage of mdx-bundler. When you actually define the component and import it's dependencies directly in MDX file (not import them from a file) everything works. So it has sth to do with imports resolution I guess. Anyways, I've created an issue in Contentlayer's repo for that: contentlayerdev/contentlayer#302

You can also test it yourself. I've added relevant tests to /content/test/components-import.md so that Contentlayer's team can reproduce the issue.

I think we should also get acquainted with Contentlayer's source code sometime, and also with the docs of mdx-bundler', which they are using under the hood, so that it's not a complete black box for us.

@olayway
Copy link
Member

olayway commented Sep 8, 2022

@khalilcodes as for the two errors related to importing TernaryPlot into an MDX file:

  1. can't resolve crypto
 > ../lib/d3/drawChart.js:1:19: error: Could not resolve "crypto" (use "platform: 'node'" when building for node)
    1 │ import crypto from "crypto";
      ╵                    ~~~~~~~~

From what I've learned, crypto is a NodeJs's built-in module, i.e. it's server-side JS only. What's interesting to me is that it works if you provide it as a global component in components object, but when you try to import it in MDX, it breaks. It makes me think that in the first case, the plot's code is run only on the server side, whereas in the second case it's also run on the client (?), thus the error. I'll do some more research on that tomorrow...
Anyways, in this case the simplest solution would be to replace node's crypto with some external dependency, like crypto-js.

@rufuspollock
Copy link
Member Author

@olayway also re crypto - that was just used for layout-hack and probably other ways to solve this.

@rufuspollock
Copy link
Member Author

@khalilcodes, @rufuspollock it seems importing components that depend on external packages doesn't work indeed 😞 I believe it has to do with Contentlayer's configuration/usage of mdx-bundler. When you actually define the component and import it's dependencies directly in MDX file (not import them from a file) everything works. So it has sth to do with imports resolution I guess. Anyways, I've created an issue in Contentlayer's repo for that: contentlayerdev/contentlayer#302

Ok, very good to know. Can we open an issue here to track this as it is important thing to track going forward.

@olayway olayway added the enhancement New feature or request label Sep 23, 2022
@rufuspollock
Copy link
Member Author

@khalilcodes @olayway is this now closable? If so can we update acceptance with a bit of detail and mark as fixed.

Also did we get to use the new components approach we want so we don't have to mess with core flowershow?

@olayway
Copy link
Member

olayway commented Oct 11, 2022

This should have been closed already. Not sure about the components approach though.

@rufuspollock
Copy link
Member Author

@olayway ok, i think doing the refactoring to use the proper component setup is the last key step. Can we check with @khalilcodes whether this got done and if not can we do this?

@rufuspollock
Copy link
Member Author

This is blocked on refactor to use components properly now that we know how to do so from #157

@khalilcodes
Copy link
Contributor

@olayway @rufuspollock

Changes for importing custom components in ecosystem

  • Ecosystem components moved to content/components and symlinked to components/ecosystem folder.
  • Profiles document type fields modified due to contentlayer errors
    • Some fields are either strings or null or numbers in the fronmatter (which was not an issue before upgrade). Resolved using field type type: "json" which can be used as a generic for any values.
  • next/links imports replaced in Hero.jsx, ProfileSearch.jsx
  • Replace imports in d3 chart function lib/d3/drawChart.js - import crypto
  • Replace import paths for all orgs/profiles - "../.contentlayer/generated/index.mjs"
  • Fix seo image handling error in components/MDX.js

All components imported directly in the markdown file now work.

Next Actions:

  • Fix eslint and prettier errors (facing conflicts between both)
  • Backport MDX.js change to flowershow for seo image error fix

@rufuspollock
Copy link
Member Author

@khalilcodes did this get done friday? is ecosystem stuff now merged and we can close this?

@khalilcodes
Copy link
Contributor

@rufuspollock This has been blocked again as the build is failing life-itself/second-renaissance#91

import of orgs/profiles from "contentlayer/generated" in markdown (which is done in component) fails as the content is not yet generated

@olayway
Copy link
Member

olayway commented Oct 25, 2022

This is done! 🧟🧨
Merged PR: life-itself/second-renaissance#91

@olayway olayway closed this as completed Oct 25, 2022
rufuspollock added a commit to rufuspollock/flowershow that referenced this issue Oct 27, 2023
… upgrade script from experience upgrading ecosystem in datopian#111.
rufuspollock added a commit to rufuspollock/flowershow that referenced this issue Oct 27, 2023
olayway pushed a commit that referenced this issue Oct 31, 2023
… script from experience upgrading ecosystem in #111.
olayway pushed a commit that referenced this issue Oct 31, 2023
… script from experience upgrading ecosystem in #111.
olayway pushed a commit that referenced this issue Oct 31, 2023
… script from experience upgrading ecosystem in #111.
olayway pushed a commit that referenced this issue Oct 31, 2023
… script from experience upgrading ecosystem in #111.
nyejon pushed a commit to nyejon/flowershow_blog that referenced this issue Nov 8, 2023
… upgrade script from experience upgrading ecosystem in datopian#111.
nyejon pushed a commit to nyejon/flowershow_blog that referenced this issue Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants