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

[WIP] Make the new module system in rdflib work #60

Merged
merged 2 commits into from
Aug 21, 2019

Conversation

megoth
Copy link
Contributor

@megoth megoth commented Aug 19, 2019

This should only be merged after linkeddata/rdflib.js#335 is merged and pushed into a new version.

Not 100% sure why this works, but seems to do the trick on my local setup.

Right, @RubenVerborgh already explained this in his comment linkeddata/rdflib.js#335 (comment)

The only breaking change is that external code that was incorrectly using import rdf from 'rdflib' should use import * as rdf from 'rdflib'. Preferably, such code imports the exposed subcomponents of rdflib, such that tree shaking becomes possible.

Copy link
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

You might be able to import * as $rdf, { NamedNode }, or might want to const { NamedNode } = $rdf.

Question is whether you really want to import * as $rdf instead of importing only the parts you need. And you probably do not want to import NamedNode, but rather import DataFactory and use its .namedNode method.

@RubenVerborgh
Copy link
Contributor

This breaking change is why I strongly suggest to remove rdflib 1.0.{1,2}.

@megoth
Copy link
Contributor Author

megoth commented Aug 19, 2019

Question is whether you really want to import * as $rdf instead of importing only the parts you need. And you probably do not want to import NamedNode, but rather import DataFactory and use its .namedNode method.

Right now it seems that there are code in solid-ui that are dependent on $rdf being in the global object window, so think we should leave it as is for now, and have as goal to remove solid-ui dependency on $rdf in the global object.

@RubenVerborgh
Copy link
Contributor

Needs an "rdflib": "^1.0.3" in package.json.

@megoth
Copy link
Contributor Author

megoth commented Aug 19, 2019

Updated to rdflib@1.0.3 in latest commit

@Vinnl Vinnl changed the title [WIP] Seems I have to do this to make the new module system in rdflib work [WIP] Make the new module system in rdflib work Aug 20, 2019
@@ -1,8 +1,9 @@
import $rdf, { NamedNode } from 'rdflib'
import * as $rdf from 'rdflib'
Copy link
Contributor

Choose a reason for hiding this comment

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

It would indeed be clearer if this would be import * as $rdf, { NamedNode } from 'rdflib' I think.

Copy link
Contributor

@timbl timbl left a comment

Choose a reason for hiding this comment

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

This seems to work when I build it and run it.
The new 1.0.3. + rdflib is supposed to be basically compatible, so long as you don't use default imports.

@timbl timbl merged commit d5c90df into master Aug 21, 2019
@timbl timbl deleted the rdflib-import-export-fix branch August 21, 2019 21:01
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.

None yet

4 participants