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

Declare Webpack loaders with require.resolve() #15299

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Declare Webpack loaders with require.resolve() #15299

merged 1 commit into from
Oct 26, 2023

Conversation

tibdex
Copy link
Member

@tibdex tibdex commented Oct 24, 2023

Fix #15298 by ensuring that loaders are resolved from @jupyterlab/builder instead of from the package being built.

This will allow to get rid of css-loader and style-loader in the devDependencies of extension-cookiecutter-ts.

By the way, worker-loader seems unused. Can we remove it?

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @tibdex

Regarding worker-loader I'm fine with its removal. But I'ld like to be cautious and to do that in a separate PR that will target the next minor version (ETA December). Would you have time to open that PR?

@tibdex
Copy link
Member Author

tibdex commented Oct 25, 2023

For worker-loader, even though it's not used directly in this repo, it looks like some extensions still rely on it: #10033 so I guess it's safer to keep it.

@krassowski
Copy link
Member

krassowski commented Oct 25, 2023

worker-loader is deprecated for webpack 5 as it supports native syntax for webworkers. Neither works well with module federation (#10197 (comment)). Also, extensions can extend the webpack configuration with custom loaders so they would still be able to use worker-loader if they choose so. But we should certainly drop it no later than for JupyterLab 5.0.

@tibdex
Copy link
Member Author

tibdex commented Oct 25, 2023

The failed CI jobs don't seem related to my changes. Is there anything left I can do to help merge this?

@krassowski
Copy link
Member

krassowski commented Oct 25, 2023

The only thing missing is deciding a milestone (4.0.x or 4.1) I am not certain how much of a potential to break things these changes have. Do you think it is safe enough change to backport it to 4.0.x or should it or would it be better for extension authors to only get it in 4.1?

While failures are indeed unrelated I restarted some tests to clear the statuses

@tibdex
Copy link
Member Author

tibdex commented Oct 25, 2023

I'm pretty sure this won't cause any regressions and can thus be safely backported to 4.0 but I don't mind waiting for 4.1 to get it.

@krassowski krassowski added this to the 4.0.x milestone Oct 26, 2023
@krassowski krassowski merged commit 311cb38 into jupyterlab:main Oct 26, 2023
76 of 81 checks passed
@krassowski
Copy link
Member

@meeseeksdev please backport to 4.0.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request Oct 26, 2023
krassowski pushed a commit that referenced this pull request Oct 26, 2023
…15307)

Co-authored-by: Thibault Derousseaux <6574550+tibdex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ModuleNotFoundError when building an extension
3 participants