[1.0] Plugin packaging file structure

[0x80]

Quoted from: #324 (comment)

On why no dist/lib folders — it's mainly because I don't want to have to rely on compiling the modules and not using a dist/lib folder means you have the option of writing straight uncompiled JS. I want to add support for sites to have a "plugins" folder where more complex sites can split up functionality into multiple plugins. We could add some sort of config in plugin's package.json to say where code is but the code for finding plugins is simpler by saying code is at the root of the plugin. I guess there's nothing preventing us from reading something out of a plugin's package.json which says where the code is at. We can't use the common convention of the index.js exporting the location of things because gatsby-node/gatsby-browser/gatsby-ssr code are all incompatible with each other given possible webpack/babel enhancements to the language.

Currently there are a lot of plugins which are defined in packages but don't follow normal package rules, where you have package.main pointing to the entry point, and exporting everything from there. I am trying to understand why the normal package.main wouldn't be sufficient.

If you want to create a plugin without compiling the source, wouldn't it be enough to point package.main to the source file and omit the build step?

Regarding incompatibility between gatsby-node/gatsby-browser/gatsby-ssr, can't we give them their own babelrc config if they differ in how they should be compiled?

If in the future you want to support a plugins folder from client code directly, I think it wouldn't be hard to make a distinction from regular packages and just read a bunch of js files and treat them as plugins. I don't see your point about "finding the sources" if you use the standard package.main for it.

To me it feels dangerous to ignore normal npm package semantics, since I don't see a good reason. Right now I see a lot of index.js which are empty with a "noop" comment. Why are they not exporting the functions and components?

I guess I might be missing the big picture, but I feel that every package could have

• all its sources in /src
• optionally compile to /dist
• point package.main to the entry point
• entry to export all functions and components.

Also the bin folders in the gatsby and gatsby-dev-cli packages should IMO just go into the src directory. Then their source can be simplified and cleaned up with babel. In the case of gatsby it can be compiled with a different babel preset to support node < 4

I was trying to improve debugging (in VSCode debugger) by generating source maps. But those source maps ended up in the packages' root. This triggered my OCD and I started cleaning the structure and cli code 😅 I won't make a PR now because I wasn't expecting this discussion, but I can finish it maybe later when it is clear to me what the structure will be.

[0x80]

Another related idea that came to mind. Afaik the prepublish script is called not only when publishing, but also after an install. If we ship the src directories of packages, and you use prepublish to trigger their build, then they compile with minimum changes for the node version available. With the latest node that would result in almost all native code.

Using the babel env plugin set to "current" it will compile the sources automatically tailored to the version of node used. I haven't tried this for published packages, but from what I understand that should work.

[0x80]

Another reason to always want to put your compiled code in a standard location like dist is that you can easily exclude it from your IDE search. I'm now searching through the repository and all kinds of double hits show up because of babel compiled artifacts.

[fabien0102]

It can be very cool to have a proper package structure definition! I'm totally agree with this idea 😄 (I'm debugging with vscode too, directly into babel output ^^)

For the entry of package (to resolve gatsby-* pattern problem), we can have a standard index.js with a simple export object.

For example:

// src/index.js
module.exports = {
  ["gatsby-node"]: require("./gatsby-node"),
  ["gatsby-ssr"]: require("./gatsby-ssr")
}

[KyleAMathews]

Regarding incompatibility between gatsby-node/gatsby-browser/gatsby-ssr, can't we give them their own babelrc config if they differ in how they should be compiled?

It's not babel it's webpack. Webpack allows you to require anything basically with its loaders which if you give that code to node.js, it chokes. SImilarly, many node.js packages can't be packaged for the web.

So we can't have an index.js that's blindly requiring everything because that'll break in node.

So either a) we have a convention that files are at the root of the plugin or b) we require config specifying a different root (e.g. dist) or c) people have to specify every plugin files location (though not by requiring).

I like the idea of when compiling locally to only compile for whatever version of node the contributor is using. Right now it's set to node v4 as that's the minimum version of node we support https://github.com/gatsbyjs/gatsby/blob/1.0/.babelrc#L5

On search — I guess I don't have that problem since my searcher (silver searcher) is setup to ignore files listed in .gitignore... I'm sympathetic to this problem since I've run into it before and it's quite annoying.

[0x80]

@KyleAMathews OK I'm getting there, but I'm not yet fully understanding the problem and implications.

Just as an example to understand things better. Say if I want to create a plugin that uses window.alert() on the browser side, and tput bel on the server side. Could you walk me through the steps in the process, and explain where that would break things?

I feel like we could do something with index.js to solve this maybe, like @fabien0102 suggested. I've seen code where there are checks for window.document or something to see if a module should be dynamically required or not. In what circumstances can we not have such conditionals checks to avoid mayhem?

And if we can't solve it with a single index entry point, would it be an option to split plugins that are not node and browser compatible into two packages? If I look at the packages now I feel like pretty much all plugins will be one or the other, so I feel like we're talking about a few exceptional cases maybe.

[KyleAMathews]

Splitting plugins is a non-starter because that's the point of a plugin — to encapsulate complex behavior in a package that matches what people want to accomplish.

E.g. gatsby-plugin-offline is a great example. It uses gatsby-node.js to generate a service worker as well as adding a special page for the AppShell. Then it uses gatsby-ssr.js to add to each generated html page the little js code snippet to load the service worker. It'd be really odd to split that into multiple plugins.

https://github.com/gatsbyjs/gatsby/tree/1.0/packages/gatsby-typegen-remark-autolink-headers/src implements ssr and browser APIs

https://github.com/gatsbyjs/gatsby/tree/1.0/packages/gatsby-plugin-glamor/src implements ssr and node APIs

Think of each gatsby-* as their own entry point. index.js is normally a no-op because you don't require the plugin directly.

Sticking with conventions makes sense if you're doing conventional things but in this case we want something very specific so I think it makes sense to break with convention (in this case, a single index.js entry point).

I would accept a PR that let you set a sub-directory as the "base" for Gatsby to look for our special entry files and then moving all the core plugins compiled output under a "dist" directory.

I think leaving the default at the root makes sense as many sites will be using the latest version of node and not use compilation and will just leave their site specific plugins "raw".

[fabien0102]

Just a question on the reason of babel everywhere,
If I well understood, our only reason to have babel, is to have flow support and async/await notation? (all others features are natively support by node 6.x (http://node.green/) and @KyleAMathews don't like to use the es6 module syntax (ref: #828 (comment))

I just ask this question because it can simplify all packages and debugging 😉 (and we can go to node 7.x to have async/await support. For flow, I need to test a little bit more to know how it's works (I personnaly prefer typescript))

[KyleAMathews]

We're supporting Node 4 until it end of lifes a year from now https://github.com/nodejs/LTS

[fabien0102]

You're right! I had retained node 6, I don't know why ^^ My bad ;)

[0x80]

I feel the source of the problem is the convention of using those fixed filenames (like gatsby-ssr.js) and depending on them directly.

What if we define the plugin's entry point like this:

module.exports = declareGatsbyPlugin(context => {
  switch (context) {
    case "ssr":
      return require("./gatsby-ssr")
    case "node":
      return require("./gatsby-node")
    case "browser":
      return require("./gatsby-browser")
  }
})

This would give back control to gatsby over what piece of code is loaded when. The entry point file could live with your other source files in the same directory, and the whole thing can be transpiled or not. The entry point would be declared in the package.main so gatsby never has to look for plugin source files itself and can just require it like a normal module.

The plugin writer can use whatever file names. The only convention is then the context names ("ssr", "browser"), and the API that each of those modules should export. But that would be no different from now of course.

The declareGatsbyPlugin function could have more parameters, for example to give the plugin a name, or other options. If you ever in the future define a new plugin API you could maybe even be backwards compatible with existing plugins by passing in a new version number.

I hope my mind is functioning properly. I have been celebrating my king's birthday 🙂

[KyleAMathews]

I guess I don't get why you think we need configuration options. Why would a plugin author want their own special name for a file? What advantage does that give them/us?

Configuration has a high cost both in additional framework complexity and in end user complexity. To justify the cost, it has to bring a lot of value. This proposal doesn't seem to be changing much other than letting people name gatsby-* as something else.

[0x80]

@KyleAMathews I don't think we need configuration options. That was just an afterthought.

What this proposal would change is that plugins can function as normal standardized npm modules, by giving the framework control over what piece of code is loaded when. It removes the need for an empty noop index.js file and utilizes package.main like any other module. This removes the need for the specific gatsby own file mapping, like having gatsby-node.js in the root of your module structure.

Additionally it gives you more flexibility in the future in terms of configuration or API versioning if the need arises, and of course you can question if this will ever be used, but isn't it preferable to have such an option anyway? We don't have to use any of it now.

Currently the plugins are formatted in a way that is specific to Gatsby, and they can't be loaded via a normal require('my-plugin') call. And because of this format there is a confusing file structure for plugins with transpiled sources, making their built artifacts appear in the root of the package, instead of a dist or lib subdirectory.

You can prevent having all these exceptions and stick to normal npm packaging rules by designing the entry point interface a little different. To me this makes perfect sense. I believe it makes your design more robust and clean, while at the same time being more predictable for plugin authors.

So ignoring my mention about configuration, what harm do you see in this proposal?

[KyleAMathews]

Ok, after thinking some more I've come around to agreeing with you. It is non-ideal to break with conventions and adding the ability for plugins to return additional information in the future is attractive.

One sticking point for this proposal is that the plugin system treat's the sites' own gatsby-* files as a "plugin" (the last plugin so it always runs last). It'd be weird to have to put an index.js in the root of a site just for this. But we can just special case loading the site's default plugin.

I also dislike needing to write more code than necessary in plugins ("never make an implementor of an API do what the framework can do") but this isn't much and making loading more explicit helps make things feel less magical which is always a good thing.

Also (mentioned this before but I'll repeat myself) the index.js file can't directly require the API files as node.js will break when trying to require window or webpack code. Instead we'll just pass back the path to the different files so they can be required when needed.

A plugin's index.js would need to look something like the following:

// index.js
return {
  node: {
    // Relative paths would automatically be resolved. You can also
    // pass an absolute path if that for some reason makes sense.
    resolve: './dist/gatsby-node.js',
  },
  ssr: {
    resolve: './dist/gatsby-ssr.js',
    options: {
      // Something something
    }
  },
  browser:
    resolve: './dist/gatsby-browser.js',
  },
}

[KyleAMathews]

A PR implementing this would need to update all the core plugins as well as update the plugin processing code to follow this. Let's put as you suggest compiled code in /dist so we can easily ignore it.

[KyleAMathews]

Would you like to take this on?

[0x80]

@KyleAMathews Nice! I'm happy we get to agree on these things.

I thought the require call inside the function was somehow already enough for things to resolve correctly for webpack and node, but I'm a little unclear on how these things work apparently.

I would take this on, but it will probably take me a week at least before I can make time. Getting into Gatsby and adopting the alpha version has delayed me a lot in getting my website released and I really have to wrap that up now asap.

I will give a heads up in this thread when I'm ready to start on this. If anyone else feels like taking this on before then please do.

[KyleAMathews]

No worries! Your help has been invaluable! Good luck on your website! (which btw, what is it?)

[0x80]

@KyleAMathews Thanks! I'm happy to help out 😄

I'll tell you later. My current one is old and I find it embarrassing in terms of copy and web development 😉

[KyleAMathews]

😆

[0x80]

Hopefully I'll have some time soon to pick this up again.

In the meantime was thinking about the api a bit more and all the different functions that are being exported. In addition to the things discussed above, I think it would be interesting to replace the module exports with explicit function calls to a gatsby api. That way we can catch wrong and deprecated use of api easily.

So instead of writing

exports.modifyWebpackConfig = ({ config, stage }) => {
}

exports.createPages = () => [
]

exports.postBuild = (args, pluginOptions) => {

}

You would write something like

const api = require('gatsby').pluginApi

api.modifyWebpackConfig(({ config, stage }) => {
})

api.createPages(() => [
])

api.postBuild((args, pluginOptions) => {

})

Here the api calls are registering functions to be called at a later time of course. The api will be able to validate the functions if needed and early in the process throw errors and warn about deprecated or removed api calls.

In terms of code I think this would not be a very big change, but it would make things more explicit, robust and user friendly.

What do you think?

[KyleAMathews]

Ooooo... yeah! I really like this idea. Yeah I've thought about doing some sort of scan on startup of what people are exporting to find errors but this is much easier. We probably want to do some sort of registering callbacks with strings to make validation easier. Maybe something like:

const api = require('gatsby').pluginApi

api.registerAPIHandler(`modifyWebpackConfig`, ({ config, stage }) => {
// ...
})

[0x80]

I agree. The naming then makes sense as well 👍

[Alxandr]

I would suggest going with actual functions rather than a single registerAPI('namehere', .... Much easier to deal with registerWebpackConfigHandler(() => ...) (or whatever naming convention you want to use. easier to deal with typing, auto discovery by supporting IDEs etc...

[0x80]

@Alxandr Agree. I didn't think about typing yet but it makes a lot of sense to make it explicit like that.