octokit / core.js

Extendable client for GitHub's REST & GraphQL APIs
MIT License
1.19k stars 310 forks source link

[BUG]: Unexpected token '{' in Safari pre 16.3 #611

Closed mellson closed 1 year ago

mellson commented 1 year ago

What happened?

Hi 👋🏻

We've been working with octokit js 3.1.0 in one of our Next.js projects, and we've stumbled upon a bit of an issue. Whenever we use the import statement, import { Octokit } from 'octokit', our application throws the error Unexpected token '{' originating from the Octokit core.

We were wondering if this could be a compatibility issue with older versions of Safari. Does anyone know if Octokit 3 is supposed to fully support these browsers? We'd really appreciate your insights!

Thanks in advance for your help!

Versions

Octokit js 3.1.0, nextjs 13.4.19

Relevant log output

No response

Code of Conduct

github-actions[bot] commented 1 year ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

gr2m commented 1 year ago

When using in browser, I'd recommend to use a smaller library. octokit comes with a lot of features and code and results in a big bundle size. Could you try if you see the same problem with @octokit/core.

You can also try importing from https://esm.sh. Try

import { Octokit } from 'https://esm.sh/@octokit/core@5'

to see if you see the same error. If you don't, it might be a problem how the imported octokit module is bundled in your setup

mellson commented 1 year ago

Thank you, @gr2m, that's very helpful input 👍🏻

I tried importing Octokit from esm.sh like this import { Octokit } from 'https://esm.sh/octokit'; And that actually works.

What I ended up on, for now, is to only import @octokit/rest in the browser instead of the full Octokit. However, that still throws the original error - but I got past that by transpiling @octokit/core. I just added it to our next config:

const nextConfig = {
...rest of config
transpilePackages: ['@octokit/core']
...
}
module.exports = nextConfig

And that works fine.

gr2m commented 1 year ago

Glad you got it working!

I hope all the compliation-related problems will go away once we fully transition to ES Modules. It will happen eventually, but we cannot give an ETA unfortunately.