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

Consider using user-supplied snowpack config for static data extraction #92

Closed
eyelidlessness opened this issue Jan 20, 2021 · 8 comments · Fixed by #94
Closed

Consider using user-supplied snowpack config for static data extraction #92

eyelidlessness opened this issue Jan 20, 2021 · 8 comments · Fixed by #94
Labels
bug Something isn't working
Milestone

Comments

@eyelidlessness
Copy link
Collaborator

This is more of a bug report than a request (like #91), but I know you’re still thinking on how to expose config. That said, I took some time to explore integration with Linaria and eventually gave up because the “special sauce” depends on runtime calls into pages, but Linaria compiles those out at build time (it errors if you miss the compile step).

In terms of how it’s a bug report, what this means is that builds simultaneously succeed (with custom config) and fail (pulling static props/paths) because the failure branch doesn’t apply any snowpack configuration to its runtime.

What this would probably look like is using a unified (or separate if you or other users would prefer) snowpack context for both the main build and the static props/paths extraction. I’m not sure how involved this would be, but I’d be happy to fork and give it a shot if you’d be interested in the contribution.

On that note, I’ve spent ... well, more time than I’d like to admit wading through the microsite internals trying to get my own setup working, and I’m very impressed with the foundation and would love to contribute in general. There’s some places I’ve had a few head scratches, mostly where some stuff in the interface is different at dev/build time or where configs for each vary significantly, but I think I have most if not all of the project in brain memory at this point and if you want another brain on this I’d love to get involved.

@natemoo-re
Copy link
Owner

Thank you for the deep dive. I would honestly love some other brains to hop in on this project since it seems to be generating a lot of interest!

I'm sure the "head scratching" spots are the same ones I'm wrestling with—the dev/build time indirection is a major issue that needs to be addressed. Snowpack didn't expose an actual SSR runtime when I wrote this, but now that it does this should be unblocked.

I need to push up my upcoming 1.1.0 changes, but I'll tag you and would love your eyes on them.

@eyelidlessness
Copy link
Collaborator Author

generating a lot of interest

Obviously I’ve already been looking at it but in case you’re not aware Microsite is prominently linked at least a couple places on Snowpack’s post-3.0 release materials. I’m sure that’s driving some traffic.

I'm sure the "head scratching" spots are the same ones I'm wrestling with—the dev/build time indirection is a major issue that needs to be addressed. Snowpack didn't expose an actual SSR runtime when I wrote this, but now that it does this should be unblocked.

Yeah, this is an area I’m particularly interested in because as far as I’m aware no existing tooling in the space specifically caters to separate client vs server/static builds, but it does look like Snowpack is going that way which is exciting. A lot of the techniques I’ve seen in your approach mirror those I’ve been trying myself (just to with less full familiarity with the tooling than I have now): a whole lot of substitution based on target environment.

I need to push up my upcoming 1.1.0 changes, but I'll tag you and would love your eyes on them.

That would be awesome. I’d really love to actually get a proof of concept of my first build up (it’s not my whole project, just a temp page so I can call my tech stack spike done, I’m happy to work on unstable APIs in the meantime) just to unblock my brain from the question of what tools I plan on using.

@natemoo-re natemoo-re linked a pull request Jan 21, 2021 that will close this issue
2 tasks
@natemoo-re natemoo-re added this to the v1.1.0 milestone Jan 21, 2021
@natemoo-re natemoo-re added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Jan 21, 2021
@natemoo-re
Copy link
Owner

I'm looking back at this issue. Am I missing something or will this be addressed in #94 with the use of a custom src/_document.tsx file? I'd love to take a look at your attempt to make sure we're covering this.

Note that I still have to implement custom _document support in dev mode, which requires dev mode SSR, which is waiting on @prefresh/snowpack to be released.

@eyelidlessness
Copy link
Collaborator Author

eyelidlessness commented Jan 21, 2021

Sorry, I'll clarify. The issue, at least as I've experienced it with 1.1.0-next.2/1.1.0-next.3 is that, while the Snowpack config is applied and works more or less as expected, the build process also imports pages to collect getStaticProps & getStaticPaths data from the staging path, where the build hasn't yet occurred. Anything in those import paths which may be compiled out is still in place and run on import.

For example with Linaria, the runtime for css calls is compiled to a class name string, and the runtime for styled is compiled out to a plain component with its class name applied as a prop. In both cases, before compilation those functions will throw if called at runtime, warning that you have forgotten the compile step. (Aside: in my opinion this isn't a wonderful DX, and it's part of why I went and looked at other libraries and settled on Fela).

It may be as simple as importing from SSR_DIR rather than STAGING_DIR (if you can trust that whatever user-supplied config isn't mangling the names you want to import), or as complex as running fetchRouteData within a Snowpack or Rollup plugin in the same build pass (which seems safer for sure).

@eyelidlessness
Copy link
Collaborator Author

I will say as a matter of opinion, I think it would be a good thing for DX generally if the entire dev and build flows were packaged as Snowpack plugins, or each as a set of plugins if there's interest in picking and choosing certain behaviors, or if flexibility with ordering of build steps is needed. That would probably eliminate the need for Microsite to muck around with a user's custom config at all, and the dev and build flow would be a bit more clear. But I'm not sure how feasible this would be, at least without some significant rework.

@natemoo-re
Copy link
Owner

Ah OK that makes more sense! Thank you. I think I have a solution for you.

As an aside, STAGING_DIR is actually the Snowpack output, so the user's config should be applied to that. SSR_DIR is generated from that output by applying Microsite's rollup magic.

I think this has something to do with your setup, which I'm happy to take a look at if you're still getting tripped up. Are you trying to hook into the server-rendered CSS in getStaticProps? There's obviously no documentation on this yet, so I totally get why that approach makes sense, but I don't think that will work (or that we want it to work).

The place for this would be in a custom src/_document.tsx file. You could use defineDocument to hook into the rendering process. prepare will be run for each page, so you can handle any global, apply-to-every-single-page logic like this.

import { Html, Head, Main, MicrositeScript, defineDocument } from 'microsite/document';

const MyDocument = ({ css }) => {
  return (
    <Html>
      <Head>
       <style data-server-styles dangerouslySetInnerHTML={{ __html: css }} />
      </Head>
      <body>
        <Main />
        <MicrositeScript />
      </body>
    </Html>
  );
};

export default defineDocument(MyDocument, {
  async prepare({ renderPage }) {
    const docProps = await renderPage();
    const css = collectServerStyles(); /* the SSR method from your library */
    return { ...docProps, css: };
  }
});

To explain why we shouldn't support this in getStaticProps or getStaticPaths:

Those methods should be isolated functions that don't have access to the Component they are rendering. This follows Next.js's Data Fetching model for familiarity and also keeps the door open for potential server-side/JIT rendering in the future. The more tangible reason fetchRouteData is run out-of-band with the actual static generation is to maximize concurrency and keep builds as fast as possible. Fetching Route Data has the potential to be a huge bottleneck, so we want to kick it off as soon as possible.

@natemoo-re
Copy link
Owner

I will say as a matter of opinion, I think it would be a good thing for DX generally if the entire dev and build flows were packaged as Snowpack plugins

I don't disagree with this at all—I'll have to explore this idea. At the time of writing, Snowpack didn't offer getServerRuntime, but I think we might be able to accomplish this through that. Microsite's build could potentially be relegated to the plugin's optimize step. I opened #99 to keep this in mind.

@eyelidlessness
Copy link
Collaborator Author

eyelidlessness commented Jan 21, 2021

I think this has something to do with your setup, which I'm happy to take a look at if you're still getting tripped up.

That's what I thought too, reading through the build. It's really baffling to me why I was getting correct Linaria behavior (it was in fact actually compiling away the runtime) but then it would still hit the runtime in fetchRouteData. I honestly gave up on this but I know you're tracking CSS-in-JS support generally so maybe we can take a look together after I get a little momentum both on my personal site and here in Microsite.

Are you trying to hook into the server-rendered CSS in getStaticProps?

Actually no, I was running Linaria's Babel/Rollup plugins at build, which is supposed to extract a static CSS file and add it to the page as a link. I tried a zillion different approaches and eventually gave up and accepted the sunk cost. So far I haven't even used getStatic* in my little spike project (though I will for my real project).

The place for this would be in a custom src/_document.tsx file. You could use defineDocument to hook into the rendering process. prepare will be run for each page, so you can handle any global, apply-to-every-single-page logic like this.

That's definitely something I'm incorporating now for a variety of reasons, and will almost certainly be a part of my solution for extracting styles.

// ...
<style data-server-styles dangerouslySetInnerHTML={{ __html: css }} />
// ...

I'm actually a weirdo 😜   who prefers a single site-wide CSS file applied with link. I'm not even fully convinced of "critical style extraction".

To explain why we shouldn't support this in getStaticProps or getStaticPaths [...]

Oh I totally agree with all of this. And I quite appreciate the fact that the API and behavior match Next.js (and this appears to be turning into a somewhat de facto standard across SSGs).

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

Successfully merging a pull request may close this issue.

2 participants