feat: add documentation for contributors, including removing system-level assumptions

globau commented 6 months ago

Feature description

While I know Javascript, I generally don't follow the JS web ecosystem so I wasn't sure how to build the add-on in order to contribute. The current CONTRIBUTING.md simply says Make your changes with zero details about the build environment or requirements,.

Here's my journey through figuring things out and working around issues encountered.

no Makefile so I assume there's an npm script I need to run. npm run seems like a reasonable place to start

  "scripts": {
    "clean": "rimraf --glob ./public/*",
    "chrome-dev": "yarn clean && yarn generate-locales && cross-env EXT_PLATFORM=Chrome node_modules/.bin/webpack --mode development --progress --config webpack/development.config.js",
…

looks like the scripts assume a lot of JS ecosystem tooling is installed globally
I'm not a fan of globally installed tooling
npx is all sorts of amazing so this looks like an easy fix: change calls such as rimraf --glob ./public/* to npx -y rimraf --glob ./public/*
after editing package.json to use npx to run yarn, rimraf, cross-env, license-report, and web-ext

$ npm run firefox-build
…
error 2fas-browser-extension@1.6.3: The engine "node" is incompatible with this module. Expected version "20.9.0". Got "21.6.2"
error Commands cannot run with an incompatible environment.

incorrect node version, looks like we need a specific version
installed nvm and installed node 20.9.0

$ npm run firefox-build
…
Error: ENOENT: no such file or directory, mkdir './public/_locales'
    at Object.mkdirSync (node:fs:1379:3)
    at /Users/byron/dev/2fas-browser-extension/webpack/utils/generateLocales.js:65:10 {
  errno: -2,
  syscall: 'mkdir',
  code: 'ENOENT',
  path: './public/_locales'
}

clearly i need to mkdir public

$ npm run firefox-build
…
No target browser changes
 Webpack PROD Build [Firefox]
Failed to load /Users/byron/dev/2fas-browser-extension/.env.
[webpack-cli] Error: Missing environment variable: API_URL

looks like build default to prod, needing an API_URL which I don't have
how about npm run firefox-dev?

$ npm run firefox-dev
…
 Webpack DEV Build [Firefox]
Failed to load /Users/byron/dev/2fas-browser-extension/.env.
[webpack-cli] Error: Missing environment variable: API_URL

ok, the url is mandatory
I assume it needs to be set in .env
cp .env.example .env and set it to http://localhost/

$ npm run firefox-build
…
 Webpack PROD Build [Firefox]
[webpack-cli] Error: Missing environment variable: WS_URL

ok, WS_URL gets http://localhost/ too

$ npm run firefox-build
…
 Webpack PROD Build [Firefox]
[webpack-cli] Error: Missing environment variable: LOCO_KEY

that's on me, I should have guessed
setting LOCO_KEY to secret.

$ npm run firefox-build
…
`sh: powershell: command not found`

makes sense, powershell is windows specific and doesn't exist on macOS
ah, the npm script has powershell Compress-Archive ./public/* ./build/firefox_ext_%npm_package_version%.zip
let's use bestzip: npx -y bestzip ./build/firefox_ext_%npm_package_version%.zip ./public/*

$ npm run firefox-build
…
Writing ./public/_locales, ./public/background, ./public/content, ./public/fonts, ./public/images, ./public/installPage, ./public/manifest.json, ./public/optionsPage to ./build/firefox_ext_%npm_package_version%.zip...
zip I/O error: No such file or directory
zip error: Could not create output file (./build/firefox_ext_%npm_package_version%.zip)

ok, mkdir build

$ npm run firefox-build
…
✨  Done in 7.37s.
Writing . to ../build/firefox_ext_%npm_package_version%.zip...
zipped!

the build completed successfully, however the archive's name is literally firefox_ext_%npm_package_version%.zip
and the paths inside the archive have a public/ prefix which is probably wrong.
sure enough, firefox couldn't find the manifest
let's try to fix the path by usingcd public && npx -y bestzip ../build/firefox_ext_%npm_package_version%.zip .

$ npm run firefox-build
…
✨  Done in 7.37s.
Writing . to ../build/firefox_ext_%npm_package_version%.zip...
zipped!

success! Firefox loaded the addon, I can start poking at it
as i don't care about the filename, I'm stopping there

For completeness here's the changes I made to the Firefox specific lines in package.json

    "firefox-dev": "npx -y yarn clean && npx -y yarn generate-locales && npx -y cross-env EXT_PLATFORM=Firefox node_modules/.bin/webpack --mode development --progress --config webpack/development.config.js",
    "firefox-prod": "npx -y yarn clean && npx -y yarn generate-locales && npx -y update-browserslist-db@latest && npx -y cross-env EXT_PLATFORM=Firefox NODE_ENV=production node_modules/.bin/webpack --mode production --progress --config webpack/production.config.js",
    "firefox-build": "npx -y yarn firefox-prod && npx -y rimraf ./public/.gitkeep && cd public && npx -y bestzip ../build/firefox_ext_%npm_package_version%.zip .",
    "firefox-run": "npx -y web-ext run --source-dir ./public/",

spoke to soon, npm run firefox-run results in errorStorageCorruptedMessage
adding --devtools to the web-ext run command shows me Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://localhost//browser_extensions. (Reason: CORS request did not succeed). Status code: (null).
I guess we need valid API and WS urls
storage.keys only contains publicKey, and is missing privateKey and extensionID
yeah SDK needs valid urls and I have no idea what they should
digging into the released add-on, API_URL=https://api2.2fas.com, using that
success!

In summary I recommend:

document that a specific version of node is required, and pointing at nvm as a solution
document that .env needs to be created and configured, including sample valid URLs (and descriptions for the 3 variables)
use npx -y to run all commands instead of assuming they are available globally
use npx -y bestzip instead of powershell Compress-Archive, which should work on any platform
automatically create necessary directories

[edit]

spoke too soon, again - pairing with my Android app doesn't work; browser is added to the app, but the add-on never proceeds to the next step
I assume this is related to WS_URL and/or LOCO_KEY
giving up now; was hoping to help fix #67

Motivation

I want to fix a bug, found zero documentation on how to setup the development environment, as well as several incorrect system-level assumptions.

Acknowledgements

[X] This issue is not a duplicate of an existing feature request.
[X] I have chosen an appropriate title.
[X] All requested information has been provided properly.

GrzegorzZajac000 commented 6 months ago

@globau Thank you for putting so much work into describing the problem. I agree that not everything was clear and the scripts were poorly prepared for external contributing. I made few changes. You can see them on branch feature/1.6.4.

FYI #67 will be fixed in next version ;)

GrzegorzZajac000 commented 5 months ago

Merged

twofas / 2fas-browser-extension