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] Add ES modules to unbuild process #796

Closed
wants to merge 3 commits into from

Conversation

rm-rf-etc
Copy link

@rm-rf-etc rm-rf-etc commented Aug 16, 2019

Opening a PR while still WIP. Wanted to make the diff easy for people to review. Relates to #789

Status

Builds and runs without error. .on() appears to work in a quick initial test. No other tests done yet.

Testing

https://github.com/rm-rf-etc/webpack-test

Misc

@amark if I change webpack to production mode, the file is 26KB (uncompressed). MacOS says gun.min.js is ~34KB.

So then I tested with closure compiler (here). I just copied the text content into the box and ran it without any changes to the build options.
From master: 12.57KB gzipped (33.19KB uncompressed)
From webpack: 9.95KB gzipped (25.16KB uncompressed)

@amark
Copy link
Owner

amark commented Aug 16, 2019

@rm-rf-etc excellently done! I'm very impressed!

If need be, I can pull this now. :)
If possible, just wanna catch up on the few changes to gun.js. For instance, in purely modular system, .on chain method needs index because .on builds on top of .get as a convenience method (it is an optional module, so it doesn't need to be index), root just has Gun constructor, meaning I predict .on will crash at "cannot call get( of undefined" if built on its own.
And I'm guessing you had to rearrange Gun.HAM = type calls because import has a weird right/left syntax?

Your unbuild updates are very clever! Love it!!

++Thanks for the esm naming. :) Makes me very happy!

IDK why Travis is complaining? Something about lodash shrinkwrap? Looks like lodash not actually used for any of your changes, can probably just delete it from optionalDeps?

@rm-rf-etc
Copy link
Author

rm-rf-etc commented Aug 16, 2019

@amark ya, still WIP.

From ESM perspective, there's no import for on.js. It seems that USE works in a fundamentally different way. The on.js code block will run in gun.js but because there's no USE('./on');, unbuild will never generate the necessary ESM import statement for on.js. You'll have to give me some input on an alternative solution.

I haven't inspected the Travis output yet. I'll remove the [WIP] once I think this is tested and ready for a better look. I wanted your input on the changes.

You're correct, ESM import statements can't be used for property assignment, so this simple change made the output work for both USE and ES modules.

I think a good solution would also evaluate how USE functions to see if we could make it more similar to ES modules so that, going forward, there's a strong alignment of the two. Without that, I'm concerned that gun will be prone to breaking because developers won't understand why something works in USE modules and not in the ESM output.

@amark
Copy link
Owner

amark commented Aug 16, 2019

Yeah, I'm all about compatibility. Very important philosophy of mine.

We'll have to catch up on the .on thing later.

@Jack-Works
Copy link

I took a look about the esm build. Rollup does not complain anything, that's great, but it seems like many of methods are missing on the Gun's prototype.

Happy to see the esm build published!

@amark
Copy link
Owner

amark commented Aug 28, 2019

👍

Keep me in the loop if you guys need anything. Let me know.

@Jack-Works
Copy link

Don't forget to add a "module" field in the package.json 🧙

@robchristian
Copy link

I know this is languishing. I promise I will come back to it. Currently struggling with webpack in Weir.

@Jack-Works
Copy link

how is that going?

@rm-rf-etc rm-rf-etc closed this Oct 19, 2019
@amark
Copy link
Owner

amark commented Oct 19, 2019

anybody else able to help with this?

@Jack-Works
Copy link

anybody else able to help with this?

Any problem on the process?

@amark
Copy link
Owner

amark commented Oct 22, 2019

@Jack-Works exports default crashes browsers unless end app developer does <script type="module, so basically the ONLY option we have is to teach ESM devs to reference build files instead of gun directly.

Or do you know of a trick that would let us change the NPM index.js to export safely? That way no browsers touch it, but build tools should.

Actually, hold up, does node support exports? my 10.16.3 is crashing on Rob's example also.

@Jack-Works
Copy link

If you specify a "module": "./es-build/index.js", webpack and rollup will respect this setting and import the file from the es-build/index.js

It is impossible to support cjs and esmodule and use no import/export declaration at the same time.

@amark
Copy link
Owner

amark commented Oct 22, 2019

@Jack-Works thanks

in the package.json or a webpack.config?

@Jack-Works
Copy link

In your package.json

@Jack-Works
Copy link

hello, any progress on this?

@Dletta
Copy link
Contributor

Dletta commented Nov 7, 2019

@Jack-Works could you try specifying "./node_modules/gun/server.js" in your webpack path, at least in node using --experimental-modules flag it imports this correctly and creates a usable version of gun

@nanotronic
Copy link

@rm-rf-etc: so, whats the problem? you are quite quiet in https://gitter.im/amark/gun recently...

@amark
Copy link
Owner

amark commented Nov 13, 2019

can someone do a PR for those webpack path thing and package.json thing? Thanks <3

@Jack-Works
Copy link

I have interest to do this when I'm free

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

6 participants