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

Provide a way for plugins to be written for slim #717

Closed
matthewp opened this Issue Jun 1, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@matthewp
Copy link
Member

commented Jun 1, 2017

We should provide some sort of mechanism for plugins, like CSS, to hook into the slim loader.

My first thought here is that the plugin should handle everything about loading. Let's say you had a module that gets transpiled to stealRequire(2) where 2 is a plugin resource like a CSS module.

Maybe we could have a configuration get included into the shim that says what plugin owns which module like:

var plugins = {
  2: 3
};

This would load 3 (if not already loaded) which would need to look like:

module.exports = function(moduleId, moduleMap){
  return new Promise((resolve) => {
    // Do whatever is needed to load moduleId
    resolve();
  });
};

Additionally I think it would be beneficial if here was a way to tell plugins to use a different module to be included in the build. This way, for example, steal-css/css.js would be for development and steal-css/slim.js (or something) would be what gets included into the production slim bundle.

@matthewp matthewp added the slim label Jun 1, 2017

@m-mujica m-mujica self-assigned this Jun 6, 2017

@m-mujica

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2017

Notes while I try to get this working:

  • Generate the steal.plugins config, maps module ids to module ids, the key being the module that requires the plugin and its value is the plugin module id.

  • Make sure the progressive version of the shim added when really needed, the current logic counts the number of bundles, but there might be 1 JS bundle and 1 CSS bundle and the "regular" loader + css support should be used

  • How to make steal-tools replace/use the "build" version of the plugin? We need the "regular" plugin up to the point where the graph is built, after that we can replace it for the one that will be called by the slim loader itself

  • The runtime css plugin should not rewrite the @import and take care of resolving the urls I think, how's that going to work? Is this already done by the plugin code that runs during the build

  • Should the plugin take the moduleId and the slimConfig object? In the css example all that the plugin needs is the css bundle address but maybe other plugins won't always load a bundle?

  • Plugin modules are loaded statically through stealRequire, I don't think we need to care about plugins being in a different bundle? maybe?

  • Having separate templates for the single/multi bundle loader won't play well with the code to add plugin support, probably need to change the API to something like Matt's originally proposed.

@m-mujica

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2017

@matthewp I pushed an example of the Css plugin in the slim loader, the loader itself does not change a lot

The plugin config you proposed here: https://github.com/m-mujica/slim-css/blob/4e17feba25d8786f923669ccbb6dd88ea81df38b/out/bundles/slim-css/index.js#L11

And the early return on the stealRequire https://github.com/m-mujica/slim-css/blob/4e17feba25d8786f923669ccbb6dd88ea81df38b/out/bundles/slim-css/index.js#L35-L39, the contract is minimal as you suggested. The loader tells the plugin all it know (the id and the config) and the plugin returns a promise.

@matthewp

This comment has been minimized.

Copy link
Member Author

commented Jun 8, 2017

Looks good! Just make sure that this is conditionally added if plugins are used.

@matthewp

This comment has been minimized.

Copy link
Member Author

commented Jun 8, 2017

I assume this is just the shim part, have you thought about how plugins will provide their shim to steal-tools?

m-mujica added a commit that referenced this issue Jun 9, 2017

m-mujica added a commit that referenced this issue Jun 12, 2017

@matthewp

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2017

@m-mujica can this be closed since #735 landed?

@m-mujica m-mujica closed this Jun 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.