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

Chore: Webclipper: Migrate build system to Webpack 5 #9670

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Jan 6, 2024

Summary

  • Upgrades app-clipper to Webpack 5
  • Fixes a runtime error in Firefox ("exports is not defined")
    • This issue may have been introduced in c06ca87
    • Optional to-do: Move to a separate pull request

Notes

To upgrade to Webpack 5, this pull request migrates away from our fork of create-react-app. create-react-app seems to be unmaintained and, for building the webclipper, a custom Webpack configuration seems to be much simpler.

Testing

  1. Add app-clipper as a development plugin (through about:debugging in Firefox)
  2. Launch Joplin in development mode and enable the webclipper API
  3. Open a website
  4. Click "Clip simplified page"
  5. Save to Joplin

This has been successfully tested in Firefox 121.0 and Chromium 120 with https://babeljs.io/docs/babel-plugin-transform-modules-commonjs

@personalizedrefrigerator personalizedrefrigerator changed the title Chore: Migrate Webclipper to Webpack 5 Chore: Webclipper: Migrate to Webpack 5 Jan 6, 2024
@personalizedrefrigerator personalizedrefrigerator changed the title Chore: Webclipper: Migrate to Webpack 5 Chore: Webclipper: Migrate build system to Webpack 5 Jan 6, 2024
@laurent22
Copy link
Owner

That looks good, thanks for updating this and getting rid of create-react-app!

@laurent22 laurent22 merged commit fc7d053 into laurent22:dev Jan 6, 2024
10 checks passed
@laurent22
Copy link
Owner

@personalizedrefrigerator, I'm not sure if this is related but I can no longer start the plugin in development mode.

On Firefox I'm getting this:

image

And on Chrome:

Could not load content scripts Error: tabsExecuteScript: Cannot load {"file":"/content_scripts/clipperUtils.js"}: Could not load file: 'content_scripts/clipperUtils.js'.
    at bridge.js:338:1
componentDidMount @ App.js:202
bridge.js:414

It looks like this file should be copied on postinstall but it's not doing it for me, even after yarn install. It may be related to the fact that yarn install has been failing for me for a few days. I will create an issue about it

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Jan 13, 2024

Try running yarn postinstall in the packages/app-clipper directory (rather than yarn install).

For me, yarn install seem to not always run postinstall (testing by adding && error to the postinstall step in app-clipper/package.json).

@laurent22
Copy link
Owner

That's true, yarn doesn't run the postinstall script when no new package has been added: yarnpkg/yarn#5476 (comment)

I think that's because of this kind of issue that I replaced most postinstall scripts by "build" scripts that can be called explicitly.

With unfixed issues like this I'm wondering if Yarn is becoming abandonware (https://github.com/yarnpkg/yarn/graphs/contributors) and whether we should start considering a different package manager.

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Jan 13, 2024

With unfixed issues like this I'm wondering if Yarn is becoming abandonware (https://github.com/yarnpkg/yarn/graphs/contributors) and whether we should start considering a different package manager.

It looks like that's the graph for the 1.x version of Yarn. The yarnpkg/berry repository seems to have more recent commits (so it seems more maintained than Yarn 1.x :) ):

screenshot: Commits continue into 2023, although there is a sharp dip near the end of 2023 coming into 2024

https://github.com/yarnpkg/berry/graphs/contributors

@laurent22
Copy link
Owner

Oh indeed, that's much better. I guess it doesn't help that Google insists on only ever showing results from classic.yarnpkg.com and yarnpkg/berry

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

Successfully merging this pull request may close these issues.

2 participants