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

Upgrade to React 18 #101

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

Conversation

spoilerdo
Copy link

Upgrade dependecies to React 18. Since modern projects need React 18

},
"peerDependencies": {
"react": "^0.14.0 || ^15.0.0 || ^16",
"react-dom": "^0.14.0 || ^15.0.0 || ^16"
"react": "^18",
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks compatibility with previous versions, as we now require at least version 18, even if there is no new functionality.

Copy link
Author

Choose a reason for hiding this comment

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

React 18 has a new way of rendering https://react.dev/reference/react-dom/render. So when upgrading to React 18 all previous version will break. I did not add this change yet to this PR, because I found this out after I made this PR.

"shx": "^0.3.4",
"webpack": "^4.25.1",
"webpack-cli": "^3.1.2"
"webpack": "^5.88.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a few major updates in there, are you sure this doesn't break anything ?
I think it would be better to separate the build tools update from the react update.

Copy link
Author

Choose a reason for hiding this comment

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

I created my own version 3 and it seems to work fine. But you can always test for yourself. I just updated the packages with a npm command. So I have not checked any versions in specific.

@felixgirault
Copy link
Contributor

Hi,
Thank you for your contribution.
There is a version 3 on the way, in which the dependencies are updated, so I don't know if I'll merge this PR as-is.
I have a question though, do you need this change because your using Orejime as a dependency, compiling it yourself ?

@spoilerdo
Copy link
Author

Yis we are using React 18 and wanted to use this package in our software. But I am not going to compile React 18 and 16 in the same project. But currently we are forking your package and adopting it to our own material UI component.

@felixgirault
Copy link
Contributor

Oh ok! I wondered why you would use Orejime like that instead of just loading the script separately.
In the upcoming version, we split the core mechanics and the interface, so it will probably be easier to do what you're doing.

@spoilerdo
Copy link
Author

Yea for now we will wait and search for a different package until version 3 is out. So keep us up to date on this version 3. We are very looking forward to it. Our customer is in desperate need of this new version haha

@felixgirault
Copy link
Contributor

Yeah it's been in the pipes for too long…
However it is more than usable right now, would you give it a try if I published an alpha version ?

@spoilerdo
Copy link
Author

Yis absolutely!! Soon we will pick up a spike to research cookie consent and we can take this within our research to test the alpha version!! So please let me know when you release it so we can research it. And we will let you know what our findings are as soon as we pick up the spike

@felixgirault
Copy link
Contributor

Great news!

I don't know if I will be able to publish a version on npm soon enough, it works ok as standalone, but there is still some work to be done to make it usable as you would.

In the meantime, here is some info to show you what you could do, to see if it would fit your use case :

In v3, the logic is separated in its own module, were you can get a "ConsentManager" object that can check cookies, enable or disable scripts etc.
Then there is the UI that is just putting visuals and controls over the manager.
You could build your very own UI using the manager, or use the build-in theming mechanism.
We're alreay using themes to provide a default interface, and one for the french state design system.
If what you want to build doesn't differ too much in terms of structure, you could create a material theme just by creating some stateless components (like here).

I hope this makes sense 😅

@felixgirault felixgirault mentioned this pull request Oct 2, 2023
@jmpp
Copy link

jmpp commented Sep 5, 2024

Note

Update : Just realized that I was confused by the opening date of that PR (4th Sept 2023). I though it was published yesterday (4th Sept 2024)

Hey there. I'm happily surprised with that PR as I was also preparing one to do the same thing. Grilled by @spoilerdo 😄

We are using Orejime in production with React 18. Having the deprecated warning gone would be great for us as well.

There is a version 3 on the way, in which the dependencies are updated, so I don't know if I'll merge this PR as-is.

Any idea for the release ? Still interested in contributions ?

Cheers

@spoilerdo
Copy link
Author

spoilerdo commented Sep 11, 2024

We created our own cookie consent by just copy pasting the logic code and tweaking it to our liking. It works fine for us and we can maintain the code for our own. So maybe thats something you can conside as well @jmpp

@jmpp
Copy link

jmpp commented Sep 11, 2024

Thanks for your answer @spoilerdo , maybe we'll consider to do the same in the future :)

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.

4 participants