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

Mutations #10

Merged
merged 52 commits into from
Feb 3, 2020
Merged

Mutations #10

merged 52 commits into from
Feb 3, 2020

Conversation

dOrgJelli
Copy link
Contributor

@dOrgJelli dOrgJelli commented Dec 24, 2019

A working prototype can be found here: https://github.com/dorgtech/the-graph-mutations-spec

rfcs/0003-mutations.md Show resolved Hide resolved
rfcs/0003-mutations.md Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
## Open Questions

- **Should the resolvers module be ES5 compliant?**
We've been operating under this assumption while developing the prototype. We have since scrapped this requirement as it has proven nearly impossible to successfully transpile our own source, along with all our dependencies, into a single monolithic module. If anyone has experience doing this I would love chat!
Copy link
Contributor

Choose a reason for hiding this comment

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

You'd think that it's pretty common and can be achieved with a transpiler and bundler, but I don't have much experience with that either. Bundling everything together into a single file at least should be no problem. Maybe ES2015 / ES6 is the better target nowadays?

Another question is the module system to use for exporting from the bundle (CommonJS, UMD, ...)?

@nenadjaja may have more experience with transpiling to older JS versions and bundling.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://www.tutorialspoint.com/babeljs/babeljs_transpile_es6_modules_to_es5.htm looks promising (webpack + babel with a single output file). I bet there are better guides out there though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks so much for this link, it was able to help me find a configuration that worked.

I've run into one last problem: building for a node environment and the browser requires different "target" types in the webpack config. I'm looking into solutions now, but I have a feeling we may end up needing to support something along the lines of:

resolvers:
  - kind: javascript/es5/node
    file: ./bundle-node/index.js
  - kind: javascript/es5/web
    file: ./bundle-web/index.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the error I receive when trying to use a "node bundle" within the browser:
image

This is because...

When running the code in the browser, there is no builtin require function. If you want to flag a module as external in the browser, you have to provide some other way to load it (put a script tag, or integrate an AMD library like require.js). Otherwise this external module cannot be imported.

Taken from this response: liady/webpack-node-externals#17 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why there even is a need to import anything. It's a bundle after all. I think I'll need some hands-on time with the bundling to figure something out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh that's very true, will investigate further.

rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
@vercel
Copy link

vercel bot commented Jan 10, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/graphprotocol/rfcs/blq8txh8i
✅ Preview: https://rfcs-git-fork-dorgtech-mutations.graphprotocol1.now.sh

rfcs/0003-mutations.md Show resolved Hide resolved
rfcs/0003-mutations.md Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
rfcs/0003-mutations.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Jannis Jannis left a comment

Choose a reason for hiding this comment

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

Alright, here we go!

@Jannis Jannis merged commit c312270 into graphprotocol:master Feb 3, 2020
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.

3 participants