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

Add support for TCFv2 and modernize the web extension #10

Open
wants to merge 148 commits into
base: master
Choose a base branch
from

Conversation

charles-tan
Copy link

  • Call TCFv2 APIs
  • Refactor of backend
  • Modernize the UI and expose more info returned in the TC string

Katie Ta and others added 30 commits October 3, 2021 21:35
Update script to use tcfv2 API's
…ie_code_flow

Fixes #4 by cleaning up code and adding listener in uCookie.js regardless if cmp locator is found
Related to Issue #6 to properly load the vendorsList
[#15] set up boilerplate for modules & hot reloading
@Perdu
Copy link
Owner

Perdu commented Jan 11, 2023

Hello,
Thanks for submitting a pull request and for the extensive work to support TCFv2.
Apologies for the delay. I have some time new to work on integrating this and pushing it to firefox and chrome stores.
However, I have a few blocking issues:

  • the first one is that the provided build command currently fails on my setup:
$ yarn run build
yarn run v1.22.19
$ node utils/build.js
[PATH]/Cookie-Glasses/utils/build.js:8
  (err) => { if (err) throw err; },
                      ^

ValidationError: Invalid configuration object. Webpack has been initialized using a configuration object that does not match the API schema.
[...]
  • Are the dependencies to yarn and webpack necessary?
  • There are a lot of dependencies in package.json. Can you justify all of them? I can't seem to find use for some of them (e.g. "acorn") in the code.
  • Can you explain why there is a web server in utils/?

The rest is minor and I can fix it later.

Thanks!

@charles-tan
Copy link
Author

Hello, Thanks for submitting a pull request and for the extensive work to support TCFv2. Apologies for the delay. I have some time new to work on integrating this and pushing it to firefox and chrome stores. However, I have a few blocking issues:

  • the first one is that the provided build command currently fails on my setup:
$ yarn run build
yarn run v1.22.19
$ node utils/build.js
[PATH]/Cookie-Glasses/utils/build.js:8
  (err) => { if (err) throw err; },
                      ^

ValidationError: Invalid configuration object. Webpack has been initialized using a configuration object that does not match the API schema.
[...]
  • Are the dependencies to yarn and webpack necessary?
  • There are a lot of dependencies in package.json. Can you justify all of them? I can't seem to find use for some of them (e.g. "acorn") in the code.
  • Can you explain why there is a web server in utils/?

The rest is minor and I can fix it later.

Thanks!

Hi, it's been quite some time since I looked at this and I've lost some context. I'll try to revisit this and look into cleaning up some dependencies. We finished this project in a bit of a hurry and I'm sure there are unused dependencies that we might have explored using and never cleaned up.

I was able to rebuild from a fresh setup. I'm using yarn version 1.22.19, npm version 9.2.0, node version v18.8.0. What versions are you using and can you provide some error logs from when you tried running yarn run build?

I recall that we used yarn because we found it easier for our dependency management, we were running into some issues with plain npm. Beyond that, I can't remember unfortunately. Is yarn something you're trying to avoid?

@charles-tan
Copy link
Author

charles-tan commented Jan 12, 2023

Webpack is not strictly necessary, but we found it to be useful for development. The webserver in utils/ is part of common chrome extension boilerplate using webpack (see https://github.com/samuelsimoes/chrome-extension-webpack-boilerplate and https://www.npmjs.com/package/chrome-extension-boilerplate-react). We try to stick to the design that the boilerplate code sets us up with. We found the interaction between scripts made more sense when we made this switch.

I apologize for the messy commit history... I tracked down the old PR where this change was implemented. There is some good context there behind our rationale for using yarn + webpack.

@Perdu
Copy link
Owner

Perdu commented Jan 18, 2023

Hello,

I have the following versions:

  • yarn 1.22.19
  • npm 8.19.2 (for some reason arch is a bit behind for this package)
  • node 19.4.0 (this also failed with node 16.19.0)

I'll come back to this a bit later, but I may remove webpack, especially since most dependencies are gone.

Don't worry about the commit history, I'll probably squish everything into a single commit when merging

@Perdu
Copy link
Owner

Perdu commented Jun 23, 2023

So, some updates as I've spent some time trying to integrate this:

  • I've tried fixing the webpack crashes (see the webpack branch) to be able to build this, but with webpack pulling over 500 dependency modules (!), I don't expect this to be maintainable
  • I've tried removing webpack altogether (see the tcfv2integration branch), but this seems too dependent on it as it's pulling @iabtcf/core as a dependency, and I don't know how to integrate this without extensive work

Honestly, I don't think I'll spend more time working on this as this project is no longer important to me.

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.

None yet

3 participants