Skip to content

⚗️(front) conditionaly install blocknote xl packages #845

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lunika
Copy link
Member

@lunika lunika commented Apr 6, 2025

Purpose

The blocknote xl packages are licensed under AGPL v3. We want to allow a reuser to build the project without them. For this, in a first step we don't list these packages in the dependencies but install them using a postinstall script.
This script is a node script looking in every package.json files under the apps directory, look if an extraDependencies key exists and if yes hten will install the listed dependencies after prompting the user if wants or not to install them.
Also an environment variable AUTO_INSTALL_EXTRA_DEPS can be used to explicitly install all the dependencies or to explicitly not install them. This will help to build an image with and without these dependencies. Moreover we also need thisin a CI context.

Screencast.From.2025-04-06.14-17-42.webm

Proposal

  • ⚗️(front) conditionaly install blocknote xl packages
  • rework the dockerfile to make it works
  • rework the CI to make it works
  • dynamically load these dependencies in the code and act if they are not present to not display the export feature.

The blocknote xl packages are licensed under AGPL v3. We want to allow a
reuser to build the project without them. For this, in a first step we
don't list these packages in the dependencies but install them using a
postinstall script.
This script is a node script looking in every package.json files under
the apps directory, look if an `extraDependencies` key exists and if yes
hten will install the listed dependencies after prompting the user if
wants or not to install them.
Also an environment variable AUTO_INSTALL_EXTRA_DEPS can be used to
explicitly install all the dependencies or to explicitly not install
them. This will help to build an image with and without these
dependencies. Moreover we also need thisin a CI context.
@lunika lunika requested review from bzg, virgile-dev and AntoLC April 6, 2025 12:14
@lunika lunika self-assigned this Apr 6, 2025
Copy link
Collaborator

@bzg bzg left a comment

Choose a reason for hiding this comment

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

My comment are just suggestions, not requested changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest install-extras.js as a more explicit name.

let shouldInstall = autoMode;
if (!autoMode) {
shouldInstall = await promptUser(`
Do you want to install ${dependencyToInstall}? (y/n):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest this:

Do you want to install ${dependencyToInstall}? (y/n): 
These packages are dual-licensed by the author of BlockNote under
AGPL-3.0 or a proprietary license. If you install them, you need to
fulfill your licensing obligations.

@@ -79,5 +77,9 @@
"typescript": "*",
"webpack": "5.98.0",
"workbox-webpack-plugin": "7.1.0"
},
"extraDependencies": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd add this comment here:

// The XL packages are dual-licensed by the author of BlockNote under
// AGPL-3.0 or a proprietary license. If you install them, you need to
// fulfill your licensing obligations.

Comment on lines +81 to +83
"extraDependencies": {
"@blocknote/xl-docx-exporter": "0.23.2-hotfix.0",
"@blocknote/xl-pdf-exporter": "0.23.2-hotfix.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to do that, neither to have a conditional-install I think , just by setting a env var at build time and have a conditional import, with the tree shaking the packages will be excluded from the final build.

import dynamic from 'next/dynamic';

let Component = null;

if (process.env.WITH_AGPL_PACKAGES === 'true') {
  Component = dynamic(() => import('blocknote-xl-package'));
} else {
  Component = dynamic(() => import('blocknote-packag'));
}

export default function Whatever() {
  return <Component />;
}

Then here, before the yarn build:

ARG SW_DEACTIVATED
ENV NEXT_PUBLIC_SW_DEACTIVATED=${SW_DEACTIVATED}
RUN yarn build

You have to add this env var:

...
ARG SW_DEACTIVATED
ENV NEXT_PUBLIC_SW_DEACTIVATED=${SW_DEACTIVATED}

ARG WITH_AGPL_PACKAGES 
ENV WITH_AGPL_PACKAGES =${WITH_AGPL_PACKAGES }

RUN yarn build
...

The final code just take what is used by the app, we don't have node_modules anymore in the image:

COPY --from=impress-builder \
/home/frontend/apps/impress/out \
/usr/share/nginx/html

Copy link
Collaborator

@virgile-dev virgile-dev left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks
Will it hide the UI associated with the feature when the person doing the yarn build says no ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants