Skip to content
This repository was archived by the owner on Feb 14, 2024. It is now read-only.

Update jupyterlite and enable file-system access #27

Merged
merged 8 commits into from
Jun 20, 2022

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Jun 17, 2022

Also use comlink for the worker communication, like jupyterlite does with Pyolite.

Fixes #26

@martinRenou
Copy link
Member Author

martinRenou commented Jun 17, 2022

@bollwyvl May I kindly ask some help on the webpack+worker setup?

Webpack seems to not be processing the worker content properly when it comes to importing comlink and drivefs in the worker.

It fails with

bootstrap:19 Uncaught (in promise) TypeError: __webpack_modules__[moduleId] is not a function
    at __webpack_require__ (bootstrap:19:1)
    at ./lib/worker.js (lib_worker_js.94424d5a23f79f89129e.js:12:65)
    at __webpack_require__ (bootstrap:19:1)
    at bootstrap:34:1
    at __webpack_require__.O (chunk loaded:23:1)
    at __webpack_require__.x (bootstrap:35:1)

Webpack generates the following code:

/******/ (() => { // webpackBootstrap
/******/ 	"use strict";
/******/ 	var __webpack_modules__ = ({

/***/ "./lib/worker.js":
/*!***********************!*\
  !*** ./lib/worker.js ***!
  \***********************/
/***/ ((__unused_webpack_module, __webpack_exports__, __webpack_require__) => {

__webpack_require__.r(__webpack_exports__);
/* harmony import */ var comlink__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! comlink */ "webpack/sharing/consume/default/comlink/comlink");
/* harmony import */ var comlink__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(comlink__WEBPACK_IMPORTED_MODULE_0__);
/* harmony import */ var _jupyterlite_contents__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! @jupyterlite/contents */ "webpack/sharing/consume/default/@jupyterlite/contents/@jupyterlite/contents");
/* harmony import */ var _jupyterlite_contents__WEBPACK_IMPORTED_MODULE_1___default = /*#__PURE__*/__webpack_require__.n(_jupyterlite_contents__WEBPACK_IMPORTED_MODULE_1__);
// Copyright (c) Thorsten Beier
// Copyright (c) JupyterLite Contributors
// Distributed under the terms of the Modified BSD License.

Weirdly, in JupyterLite, the way you set things up it does not generate something so similar:

"use strict";
(self["webpackChunk_JUPYTERLAB_CORE_OUTPUT"] = self["webpackChunk_JUPYTERLAB_CORE_OUTPUT"] || []).push([["packages_pyolite-kernel_lib_worker_js"],{

/***/ 88289:
/*!************************************************!*\
  !*** ../packages/pyolite-kernel/lib/worker.js ***!
  \************************************************/
/***/ ((__unused_webpack_module, __webpack_exports__, __webpack_require__) => {

__webpack_require__.r(__webpack_exports__);
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
/* harmony export */   "PyoliteRemoteKernel": () => (/* binding */ PyoliteRemoteKernel)
/* harmony export */ });
/* harmony import */ var _jupyterlite_contents__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! @jupyterlite/contents */ 39694);
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

Do you know what I could be missing? I tried using the worker-loader manually but failed to... I don't understand what you've done differently in JupyterLite when doing this.

@martinRenou
Copy link
Member Author

Nevermind @bollwyvl, @trungleduc was able to help me. For some reason, adding a separate webpack setup for the worker fixes it.

Starting to have drive support in xeus-python as well 💯

yeah

@DerThorsten
Copy link
Collaborator

DerThorsten commented Jun 17, 2022

@martinRenou does it work to have a some fubar.py and import it if we add /drive/test to the pythonpath?
If so, we should add it to the pythonpath by default

@bollwyvl
Copy link

bollwyvl commented Jun 17, 2022 via email

@martinRenou
Copy link
Member Author

@DerThorsten idk! But we should definitely try that and see if that works.

@bollwyvl yeah.. I'd like to get rid of the TS-loader before merging. We should not need it.

@bollwyvl
Copy link

bollwyvl commented Jun 17, 2022 via email

@martinRenou
Copy link
Member Author

martinRenou commented Jun 20, 2022

@martinRenou does it work to have a some fubar.py and import it if we add /drive/test to the pythonpath?
If so, we should add it to the pythonpath by default

Relative imports does work indeed! So we should add /drive/test to the pythonpath We actually don't need to do that I guess

@DerThorsten
Copy link
Collaborator

Jeah,you are right, relative imports are the proper thing in this case

@martinRenou martinRenou marked this pull request as ready for review June 20, 2022 13:08
@martinRenou martinRenou merged commit 3317661 into jupyterlite:main Jun 20, 2022
@martinRenou martinRenou deleted the filesystem branch June 20, 2022 14:12
@martinRenou martinRenou added the enhancement New feature or request label Jun 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement new DriveFS
3 participants