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

Removed dependency to global $rdf #159

Closed
wants to merge 0 commits into from
Closed

Conversation

m-emm
Copy link
Contributor

@m-emm m-emm commented Dec 22, 2016

When including rdflib.js into a web application using webpack, the
resulting application doesn't run. The reason for this is that there are
references to the global $rdf object in modules which are initialized
before the global $rdf object has been properly initialized.

Removed all references to the global $rdf object and
replaced them by direct references to the underlying functionality.

@dmitrizagidulin
Copy link
Contributor

@megemege77 - Great catch on the usage of the $rdf global. Thanks!

@@ -37,6 +37,7 @@ const parseRDFaDOM = require('./rdfaparser').parseRDFaDOM
const RDFParser = require('./rdfxmlparser')
const Uri = require('./uri')
const Util = require('./util')
const serialize = require('./serialize')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you're adding const serialize = require('./serialize') to fetcher.js because you want to fix the usage of $rdf.serialize on line 590?
Do you mind adding that fix to this PR?
(Also, while you're in fetcher.js, can you also fix the global usage of $rdf.Util on line 867?)

@m-emm
Copy link
Contributor Author

m-emm commented Dec 26, 2016

Dear Dmitri

Thanks for the constructive feedback, I'm glad you like the idea of removing the reference to the global. I reworked it to really remove all references, as you suggested, but did it in one comit in a new PR, see linked PR #161.

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.

2 participants