souvikinator / notion-to-md

Convert notion pages, block and list of blocks to markdown (supports nesting and custom parsing)
https://www.npmjs.com/package/notion-to-md
MIT License
1.08k stars 89 forks source link

Invalid RegEx on Safari #76

Closed DevMattDavies closed 1 year ago

DevMattDavies commented 1 year ago

Works fine on Chrome and Firefox, but on Safari I receive the following unhandled runtime error when accessing the project through localhost:

'SyntaxError: Invalid regular expression: invalid group specifier name'

File with issue: ./node_modules/notion-to-md/build/utils/md.js

notion-to-md v2.5.5

souvikinator commented 1 year ago

Thanks for reporting. I'll try to reproduce it locally and reach out to you.

souvikinator commented 1 year ago

@DevMattDavies Can you share the problematic content that may have triggered this error? Or can you simply share your notion page so that I can test it with the solution locally?

bmitchinson commented 1 year ago

I'm also seeing this. It looks like it's due to an internal usage of regexs that utilize look behinds.

Looks like it's fixed on iOS 16.4 potentially, but that's too recent of a release for me to rely on :( https://stackoverflow.com/questions/51568821/works-in-chrome-but-breaks-in-safari-invalid-regular-expression-invalid-group

Here's the page I'm experiencing this on: https://home-mitchinson-dev-git-safari-bug-bmitchinson.vercel.app Here's the source code: https://github.com/bmitchinson/home-mitchinson-dev/blob/main/src/pages/index.tsx#L55

Including that line and importing the package white screens the page.

Only effects mobile iOS Safari Versions <16.4 and not safari 16.4 desktop.

bmitchinson commented 1 year ago

Edited link

souvikinator commented 1 year ago

Thanks for sharing the links, I'll test it. The issue is fixed and it should be live in few days

souvikinator commented 1 year ago

v2.6.0 is live now. It should fix the issue. Can you confirm from your end?

DevMattDavies commented 1 year ago

Thanks for looking into this - have updated to v2.6.0 but now getting a compilation error (affects all browsers), seems to be related to the fs-extra lib:

`./node_modules/graceful-fs/graceful-fs.js:1:0 Module not found: Can't resolve 'fs'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module: ./node_modules/fs-extra/lib/fs/index.js ./node_modules/fs-extra/lib/index.js ./node_modules/notion-to-md/build/notion-to-md.js ./node_modules/notion-to-md/build/index.js ./src/utils/getAbbreviatedPosts.ts ./src/components/HomePage/HomePage.tsx ./src/pages/index.tsx`

souvikinator commented 1 year ago

Seems like you are using it in the frontend. File system API only works on the server. Correct me if I'm wrong.

Are you using the whole Library including the notion SDK in frontend?

DevMattDavies commented 1 year ago

The getAbbreviatedPosts function (in which notion-to-md is being used) is used in the getStaticProps function of next, so should be running server-side only.

souvikinator commented 1 year ago

Can you point to the code and the function you are referring to. I couldn't find it in the above provided links.

DevMattDavies commented 1 year ago

Sorry I think that was another developer who linked their github/website above.

getAbbreviatedPosts function here:

https://github.com/DevMattDavies/mrgdavies-website-revised/blob/main/src/utils/getAbbreviatedPosts.ts

(edit): The getStaticProps function where the above function is being used can be found here:

https://github.com/DevMattDavies/mrgdavies-website-revised/blob/main/src/components/HomePage/HomePage.tsx

For clarity the package was working as intended before v.2.5.5, it was just that specific update that broke regex on safari, and then this new issue introduced with v.2.6.

souvikinator commented 1 year ago

Alright so I'm not sure why it's not working with getStaticProps but next removes any such imports during build.

Can you follow this answer and let me know if it works? https://stackoverflow.com/a/68098547

DevMattDavies commented 1 year ago

Tried adding the webpack config to next.config.js, now getting new error:

TypeError: undefined is not an object (evaluating 'fs.realpath.native')

souvikinator commented 1 year ago

alright! I'll remove the dependency on fs and leave that on users. Should be fixed in the next release. can you create a new issue and close this one?

DevMattDavies commented 1 year ago

New issue opened - current issue being closed by request.