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

Use Isolate-version of ScriptOrigin constructor #486

Merged
merged 2 commits into from
Jun 1, 2022

Conversation

stesie
Copy link
Member

@stesie stesie commented May 31, 2022

The new constructor was introduced with V8 9.0 and e.g. 10.1 doesn't have the old one any longer.

So in order to support 10.x versions we have to support the new version. And since 9.0 is pretty old already I've decided not to support both, but drop support for version < 9.0

@stesie stesie force-pushed the script-origin-with-isolate branch from b8e8e6d to 7247e9f Compare June 1, 2022 06:08
@stesie stesie changed the base branch from php8 to php7 June 1, 2022 06:08
@stesie
Copy link
Member Author

stesie commented Jun 1, 2022

Rebased this PR so PHP 7.4 also profits from it.

fixes #488

@stesie stesie linked an issue Jun 1, 2022 that may be closed by this pull request
@stesie stesie merged commit 0754d73 into phpv8:php7 Jun 1, 2022
@redbullmarky
Copy link
Collaborator

@stesie i've not tried yet, but does V8Js work fully with V8 10+ otherwise?

@stesie
Copy link
Member Author

stesie commented Jun 1, 2022

@redbullmarky I've tried with 10.1.124.12 yesterday (with these changes here + php8.1) and it worked just fine. Tested on MacOS w/ arm64 (M1 chip)

After all two tests failed, amongst it the new timezone test, which produced "slightly" different output there.
The extension itself worked well enough that I didn't care enough for those 😇
... cannot tell if it's a MacOS thing that existed before. TBH yesterday was the first time I've tried on the Mac at all 🙂

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.

error: no matching constructor for initialization of 'v8::ScriptOrigin'
2 participants