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

Require 'package' instead of 'package/' so webpack activates sharing #9300

Merged
merged 1 commit into from Nov 9, 2020

Conversation

jasongrout
Copy link
Contributor

References

This addresses the underlying problem in at least part of #9289.

Code changes

Webpack module federation was not creating shared modules for the direct extensions, only for their dependencies. For example, @jupyterlab/application was packaged as a shared module, but @jupyterlab/application-extension was not.

The reasoning here is a bit subtle. The problem came down to how these top-level extensions were imported. The template made the require statements similar to @jupyterlab/application-extension/, with a trailing slash (which was nominally a slash between the package name and the actual import in the package, but mostly that actual import was an empty string, so we’d just be left with a trailing slash). However, webpack interpreted that trailing slash as indicating the import was only for paths starting with the package name, and not as a top-level package import. This meant that webpack did not see that these modules were being used, so did not include them in the list of modules packaged as shared modules. In other words, these top-level imports were treated as this case:

https://github.com/webpack/webpack/blob/7415a618460795874fdf95c8f03363ea96709d52/lib/sharing/ProvideSharedPlugin.js#L97-L99

instead of as this case:

https://github.com/webpack/webpack/blob/7415a618460795874fdf95c8f03363ea96709d52/lib/sharing/ProvideSharedPlugin.js#L100-L102

The fix here is simple - only put the slash in the require statement if we actually have a path to import.

User-facing changes

Backwards-incompatible changes

This addresses the underlying problem in at least part of jupyterlab#9289.

Webpack module federation was not creating shared modules for the direct extensions, only for their dependencies. For example, @jupyterlab/application was packaged as a shared module, but @jupyterlab/application-extension was not.

The reasoning here is a bit subtle. The problem came down to how these top-level extensions were imported. The template made the require statements similar to @jupyterlab/application-extension/, with a trailing slash (which was nominally a slash between the package name and the actual import in the package, but mostly that actual import was an empty string, so we’d just be left with a trailing slash). However, webpack interpreted that trailing slash as indicating the import was only for paths starting with the package name, and not as a top-level package import. This meant that webpack did not see that these modules were being used, so did not include them in the list of modules packaged as shared modules. In other words, these top-level imports were treated as this case:

https://github.com/webpack/webpack/blob/7415a618460795874fdf95c8f03363ea96709d52/lib/sharing/ProvideSharedPlugin.js#L97-L99

instead of as this case:

https://github.com/webpack/webpack/blob/7415a618460795874fdf95c8f03363ea96709d52/lib/sharing/ProvideSharedPlugin.js#L100-L102

The fix here is simple - only put the slash in the require statement if we actually have a path to import.
@jasongrout jasongrout added this to the 3.0 milestone Nov 7, 2020
@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@jasongrout
Copy link
Contributor Author

To check this manually:

conda create -y -n jlab-9289 python nodejs
conda activate jlab-9289

pip install --pre jupyterlab
git clone https://github.com/jasongrout/provider.git
git clone https://github.com/jasongrout/consumer.git
cd provider
npm install
jupyter labextension install .
cd ..

cd consumer
jlpm add <ABSOLUTE PATH TO ../provider> # or manually edit package.json to give the absolute path to provider
pip install .

Check things with jupyter labextension list.

Now do jupyter lab and notice that there are errors in the console, and not the expected logs of JupyterLab extension provider is activated!, JupyterLab extension consumer is activated!, Provider token is PROVIDER_STRING.

Now edit site-packages/jupyterlab/staging/index.js to apply #9300:

pushd <PATH TO site-packages/jupyterlab/staging>
curl https://patch-diff.githubusercontent.com/raw/jupyterlab/jupyterlab/pull/9300.patch | patch -p2
popd
jupyter lab build
jupyter lab

Things should work now (no relevant errors, console logs as above).

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@blink1073 blink1073 merged commit 8b2c21f into jupyterlab:master Nov 9, 2020
1 check was pending
@jtpio
Copy link
Member

jtpio commented Nov 11, 2020

Wondering what would happen in the case where the Token is exported from a different package, and both consumer and provider would import it. That would mean 3 packages with the token shared in a "middle" package:

  • token: exports the token:
export const IProvider = new Token<string>(
  'provider:plugin'
);
  • provider: provides the token:
import { IProvider } from 'token';

const extension: JupyterFrontEndPlugin<IProvider> = {
  id: 'provider:plugin',
  provides: IProvider,
  autoStart: true,
  activate: (app: JupyterFrontEnd): IProvider => {
    console.log('JupyterLab extension provider is activated!');
    return 'PROVIDER_STRING';
  }
};
  • consumer: consumes the token:
import {IProvider} from 'token';

const extension: JupyterFrontEndPlugin<void> = {
  id: 'consumer',
  autoStart: true,
  requires: [IProvider],
  activate: (app: JupyterFrontEnd, provider: IProvider) => {
    console.log('JupyterLab extension consumer is activated!');
    console.log(`Provider token is ${provider}`);
  }
};

Still with:

  • provider installed as a compiled extension (jupyter labextension install .)
  • consumer: installed as federated

@jtpio
Copy link
Member

jtpio commented Nov 11, 2020

Wondering what would happen in the case where the Token is exported from a different package, and both consumer and provider would import it

Note to self: check #9310 as this should address this use case.

@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 May 11, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 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 this pull request may close these issues.

None yet

3 participants