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

Custom webpack.config.js for federated extensions #9175

Closed
jtpio opened this issue Oct 17, 2020 · 9 comments · Fixed by #9224
Closed

Custom webpack.config.js for federated extensions #9175

jtpio opened this issue Oct 17, 2020 · 9 comments · Fixed by #9224
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@jtpio
Copy link
Member

jtpio commented Oct 17, 2020

Since the config used to build a federated extension is generated here:

function generateConfig({

It looks it should be possible to let the extension author specify additional options.

For example specifying experiments: https://webpack.js.org/configuration/experiments

Maybe there could be the possibility to do so, for example by loading a webpack.config.js local to the extension, and merging it with the rest of the generated config?

Something like this:

diff --git a/builder/src/extensionConfig.ts b/builder/src/extensionConfig.ts
index 9db107782..a7bda7ab0 100644
--- a/builder/src/extensionConfig.ts
+++ b/builder/src/extensionConfig.ts
@@ -194,6 +194,11 @@ function generateConfig({
     }
   }

+  const customConfigPath = path.join(packagePath, 'webpack.config.js');
+  let customWebpackConfig = {};
+  if (fs.existsSync(customConfigPath)) {
+      customWebpackConfig = require(customConfigPath);
+  }
   const config = [
     merge(baseConfig, {
       mode,
@@ -220,7 +225,7 @@ function generateConfig({
         }),
         new CleanupPlugin()
       ]
-    })
+    }, customWebpackConfig)
   ].concat(extras);

   if (mode === 'development') {
@echarles
Copy link
Contributor

I like the idea for the developer to be able to customize webpack. With the proposed diff, it looks that webpack.config.js would be read from the package folder. Any option to also have a common place where that custom webpack config would be applied for all the packages?

@jasongrout
Copy link
Contributor

Perhaps the location should be another item in the package.json jupyterlab key?

@jasongrout
Copy link
Contributor

This does mean that we would essentially be making webpack an official part of the api, which is quite a change from our original direction of not exposing that as an implementation detail. That is an important distinction and decision. Is there a specific usecase in mind, or is this a theoretical usecase?

@jtpio
Copy link
Member Author

jtpio commented Oct 18, 2020

This does mean that we would essentially be making webpack an official part of the api

Yes that's probably the main issue.

Is there a specific usecase in mind, or is this a theoretical usecase?

This repo for example bundles local WebAssembly modules and uses top level await: https://github.com/jtpio/jupyterlab-wasm-example

And with webpack 5:

Note that this also means WebAssembly support is now disabled by default.

https://webpack.js.org/blog/2020-10-10-webpack-5-release/#experiments

So having a way to hook into the webpack config to add the following experiments is quite convenient:

{
  experiments: {
      topLevelAwait: true,
      asyncWebAssembly: true,
  }
}

As an alternative, extension authors could probably build their own builder on top of @jupyterlab/builder. But that might still require passing a custom webpack config or overriding the base: https://github.com/jupyterlab/jupyterlab/blob/90b58a8e88d0113b75d5bbc84b2083c712ae8f76/builder/src/webpack.config.base.ts. And also telling jupyter labextension build to use a different builder.

Or patch @jupyterlab/builder in place with something like this: https://github.com/jtpio/jupyterlab-wasm-example/blob/main/fix-config.js

This is also similar to this issue: #4328. (but giving control on the whole webpack config and not just the rules). Also now with federated extensions this wouldn't affect the global lab config, only the one for the federated extension. But would still make webpack part of the api.

@echarles
Copy link
Contributor

This does mean that we would essentially be making webpack an official part of the api

Yes that's probably the main issue.

Would we provide this feature to extension authors only, or extension+core?

If we provide it only to extension authors, the risk or decision trade-off to use those specific webpack configs can be clearly documented and the author should know what he/she does.

@jtpio
Copy link
Member Author

jtpio commented Oct 21, 2020

Adding to the 3.0 milestone for consideration (even if end up deciding not to do anything about it for 3.0).

@jtpio
Copy link
Member Author

jtpio commented Oct 21, 2020

Would we provide this feature to extension authors only, or extension+core?

That would only apply to federated extension (with the build process almost entirely managed by @jupyterlab/builder).

If we provide it only to extension authors, the risk or decision trade-off to use those specific webpack configs can be clearly documented and the author should know what he/she does.

But that still means that webpack will be part of the API we expose to extension authors? And that it would be marked as experiment or "you're on your own" in the documentation.

@jasongrout
Copy link
Contributor

jasongrout commented Oct 24, 2020

I've thought about this for a bit. I'm okay with going forward here, in the spirit of making it possible for developers to experiment more easily, if we move quickly to get this in (i.e., so we can have at least one RC with it in to test it, and we don't delay the final release):

I think for implementation details:

  • have a subkey of the jupyterlab config in the package.json that lists a location for a webpack config that we would merge our webpack config into (so it would override our webpack config).
  • Document this very clearly as an experimental api, that it may change or be eliminated in the future, and that it is very easy to mess things up if you override parts of the system config.

@jtpio - do you want to propose a PR we can more specifically discuss? I realize my opinion is only one opinion, but I think if we are going to try to have this as even a possibility for 3.0, we need to have a specific PR we can discuss since the timeline is so short. I have hope since there were no huge objections raised to the main question in the dev meeting or on this issue in the last week.

@jtpio
Copy link
Member Author

jtpio commented Oct 24, 2020

Just opened a draft PR (still incomplete, will continue early next week): #9224

@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Apr 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants