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 %external() class feature detection example #361

Conversation

kevanstannard
Copy link
Contributor

Fixes #351

As discussed in the issue.

Should be editable, feel free to edit

@ryyppy
Copy link
Member

ryyppy commented May 15, 2021

\cc @chenglou

@kevanstannard
Copy link
Contributor Author

Hi @chenglou @ryyppy just following up.

I'm also mindful this is using the \"" syntax ( referring to #242 and rescript-lang/rescript#5269 )

Any preference on next steps for this issue?

@bobzhang
Copy link
Member

bobzhang commented Sep 2, 2021

For this particular feature request, I am fine to generalize it so user can write %external("AudioContext").
Is it generally useful & what's the benefit compared with using raw directly?

@kevanstannard
Copy link
Contributor Author

kevanstannard commented Sep 2, 2021

Thanks @bobzhang

If we use raw, do you mean something like this?

type audioContext
@new external createAudioContext: unit => audioContext = "AudioContext"

let canUseAudioContext: bool = %raw(`typeof AudioContext === "function"`)
let audioContext = canUseAudioContext ? Some(createAudioContext()) : None

I personally would be fine to use that, but I'm happy to be guided by the team.

Just a little feedback on %external(). I was a bit confused how it worked at first, since %external(a) worked, but %external(A) didn't work, and %external("A") didn't work and %external(\"A") did work. From this I realised that the naming is based on using valid identifiers.

Was your suggestion to generalize mean that we would always using a string? E.g. %external("__DEV__"), %external("a"), %external("A")? That sounds like a helpful improvement, and also avoids using \"".

If that's the case, would it be better to not document this for now?

@bobzhang
Copy link
Member

bobzhang commented Sep 3, 2021

Was your suggestion to generalize mean that we would always using a string?
Yes

In my opinion, external here only has marginal benefit over using raw, so maybe we should stick to raw for simplicity

@kevanstannard
Copy link
Contributor Author

Thanks @bobzhang, in that case let's close this PR.

@kevanstannard kevanstannard deleted the external-class-feature-detection-example branch September 4, 2021 19:26
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.

Document %external(\"SomeValue")
3 participants