Closed davidroeca closed 6 years ago
Thank you for this! I'll probably only be able to look into this on Saturday.
@aadsm thanks, no rush!
I figured out how to ignore specific modules for the react-native
bundler by adding the following to package.json:
{
...
"dependencies": {
...
},
"react-native": {
"fs": false
},
...
}
Completely undocumented, but found out that functionality on SO.
Also apparently checking that the global navigator
is set, and checking that navigator.product === "ReactNative"
is the way to check for react-native
at runtime.
So the issues I've listed above should be non-issues and this should (hopefully) not break a browser or nodejs
build.
Still need to look into testing though, at the very least a few mocks.
I’m slightly concerned about the undocumented features. Let me ask the React Native team tomorrow to see what’s their advice.
Makes sense, looking forward to their input. I also realize that this worked well with my application ignoring the node_modules/jsmediatags/build2/NodeFileReader.js
in .babelrc
. With that line removed, the react-native
bundler still fails.
Edit: Adding an additional line "./NodeFileReader.js": false
fixes this. Isn't the most elegant though.
hey! finally found some time to look into this.
peerDependencies
a bit better? I haven’t done RN development myself so not sure what’s about.react-native
package in devDependencies
? I don’t see it being required anywhere.atob
to convert from base64.@aadsm sorry for the late reply!
peerDependencies
here since the dependencies are only required for the react-native tag reader. An alternative bundle as a separate package for react-native
(e.g. jsmediatags-rn
) would make this unnecessary. We don't want people to have to install react-native-fs
unless they're using react-native
.react-native
would be necessary in devDependencies
if an integration test required it.atob
would need to be required from a third-party module since it isn't supported as a global in react-native
. The Buffer
polyfill supports this out of the box.Hi @davidroeca,
Sorry for the delay, I’ve been a bit back and forth with the RN team and yeah, they don’t recommend using undocumented features. However, your changes seem simple enough to understand or fix in the future if needed and I prefer to have this than no support at all for RN in the library.
Thank you so much for this!
Would you be willing to write a blog post (or something) about using jsmediatags with React Native?
@aadsm this PR adds a "working" example with
react-native
.I've been able to use the tag reader in my
react-native
app with the following manual steps:build2/
as built.build2/
, removeNodeFileReader.js
and all references toNodeFileReader
injsmediatags.js
.This PR should be ready to merge once the following is handled or at least considered:
optionalPeerDependencies
and not sure if there is a better approach.jsmediatags
,jsmediatags-node
,jsmediatags-rn
)--this approach probably handles 1 and 2; could consider webpack too.See issue #82