Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Publish on NPM #5

Closed
alshakero opened this issue Mar 3, 2017 · 20 comments
Closed

Publish on NPM #5

alshakero opened this issue Mar 3, 2017 · 20 comments

Comments

@alshakero
Copy link
Collaborator

This issue applies to JSON-Patch-OT, JSON-Patch-Queue, and JSON-Patch-OT-agent.

Because many people rely on these repos and use files in src directly. And because we learned the hard way with fast-json-patch that a Node module that supports TS and ES6 imports and runs in the browser without bundling is buggy (here: Starcounter-Jack/JSON-Patch#140). My preliminary plan is to do the following:

  1. Keep the files in src untouched, logic wise. And emit a warning asking people to use the bundle in dist folder. With the option to use a minified bundle.
  2. Create a new, temporary dir, called newsrc, and put the Node friendly files there. Bundle them with webpack if needed, and put their compiled Browser + Node friendly versions in dist. And also make sure to throw a warning in these files to use dist folder in production. These files shouldn't be used at all except for development. Everything is in dist.

By the time all people start using dist folder, nobody will be using neither src or newsrc. And this will give us the freedom to remove the old files, and rename our new folder to src and live happliy ever after.

WDYT @tomalec @warpech?

@warpech
Copy link
Contributor

warpech commented Mar 3, 2017

Because many people

Who is this many people? Current Starcounter users? Or external community members?

I suppose that the community of people from outside of Starcounter who use JSON-Patch-OT, JSON-Patch-Queue, and JSON-Patch-OT-agent is only a fraction of the community of fast-json-patch users. So we don't have to care that much about smooth transition here.

My opinion is to just break the backwards compatibility and increment the major number in JSON-Patch-OT, JSON-Patch-Queue, and JSON-Patch-OT-agent.

@alshakero nevertheless I appreciate the community first approach, this should be our default consideration always.

@tomalec WDYT?

@alshakero
Copy link
Collaborator Author

@warpech well everywhere Puppet is used in Starcounter or otherwise, these repos are referenced. And I think it's a win-win situation. There is no harm at all of this transition. After one month we can simply delete src folder and rename newsrc to src without any problems and with a clean conscience, so to say 🤣

@warpech
Copy link
Contributor

warpech commented Mar 3, 2017

Starcounter comes with a copy of JSON-Patch-OT 1.0.1, so current Starcounter users do not care what we do in JSON-Patch-OT 2.0.0

Future Starcounter users also do not care what we do with JSON-Patch-OT 2.0.0, because it will be bundled in Palindrom 3.0.0

What's the fallacy in my reasoning?

@alshakero
Copy link
Collaborator Author

alshakero commented Mar 3, 2017

Hmmm you're right. I'll fix and test my PRs.

@tomalec if you agree, don't bother with reviewing for now.

@warpech
Copy link
Contributor

warpech commented Mar 9, 2017

Are the dist directories needed in JSON-Patch-OT, JSON-Patch-OT-agent, JSON-Patch-Queue and JSONPatcherProxy?

I was thinking that in these repos we can live with just the NPM way of defining exports. These libraries are unlikely to be used directly in a web browser.

@alshakero
Copy link
Collaborator Author

@warpech You're right. But these files need zero maintenance and serve a legit purpose.

@warpech
Copy link
Contributor

warpech commented Mar 10, 2017

But these files need zero maintenance and serve a legit purpose.

How come they are zero maintenance? Don't they require a building script? If I understand properly, if all we have is src and test, then we don't need any Babel or Webpack. If we have dist then we need Babel or Webpack.

@alshakero
Copy link
Collaborator Author

@warpech I honestly don't mind eitherway. I just think it's non-code work and all automated. If one person could use it, I wouldn't mind taking care of it. Especially that the work is done now and removing it to save time is actually time consuming.

If you think they're trouble, just thumbs up this and I'll remove them.

@warpech
Copy link
Contributor

warpech commented Mar 10, 2017

I that removing this is time saving in future, because there will be one piece less to worry about. Let's see what @tomalec has to say on this.

@tomalec
Copy link
Member

tomalec commented Mar 10, 2017

runs in the browser without bundling is buggy

I wouldn't say running w/o bundling was buggy, as it was working perfectly for years. Lack of bundling was not the problem, the problems started mixing TS, NPM modules, and serve the same thing for the browser.


In general I do agree with @warpech. I also like to see clean repo with source files + spec only. That is the only think I am interested in, as a maintainer as well as a user of the module. Everything else is blurring the picture of what it is all about and complicating the development process.

Therefore, I would like to se the source fill as obfuscated as possible, and possible to be the same file I could load to the browser, or my node app.
I think, few lines as those are quite fine to achieve that https://github.com/Palindrom/JSON-Patch-OT/blob/master/src/json-patch-ot.js#L152.

I don't think we should bother much about delivering minified files and I don't like using Webpack.
If we will deliver just bare source file, consumers of this tiny module could still use any minifying/bundling/packing/browserifying/vulcanizing tool of their choosing.
By using any of those, we are locking ourselves with it, adding maintenance overhead (even single cli call), get into the trouble of version of this tool, or migrating to new more fancy one.

During my overall Starcounter development, I would really like to be able to attach every single dependency as a non-minified file using separate <script> tag. It really helps to make the debugging more modular.

Maybe we could consider doing it similar to what Polymer team did some time ago: releasing just minified bundle of entire stack (in our case Palindorm+deps) in version bump commit - which was a outstanding leaf from master branch.

Or go even further and finally do such bundling on Stracounter level, so it would bundle entire Palindrom stack it uses, core CE definitions, and polyfills together, and by single flag I would be able to "unbundle" it for debugging.

@tomalec
Copy link
Member

tomalec commented Mar 10, 2017

BTE, speaking of OP, @alshakero could you add me and @warpech as collaborators to all json patch packages you published https://www.npmjs.com/search?q=json+patch+ot ?

Maybe we could create Palindrom organization in NPM as well?

@alshakero
Copy link
Collaborator Author

Fair points. I'll take action.

could you add me and @warpech as collaborators

done

Maybe we could create Palindrom organization in NPM as well?

Then we'll have to pay.

@tomalec
Copy link
Member

tomalec commented Mar 10, 2017

Then we'll have to pay.

For open-source? I thought it's free.

So maybe we could just create a Palindrom user? ;)

@alshakero
Copy link
Collaborator Author

@tomalec I didn't know but @warpech and I discussed it a month or so ago and he told me an org is paid and it would be smart to publish in individual accounts.

https://www.npmjs.com/org/create

@tomalec
Copy link
Member

tomalec commented Mar 10, 2017

Yes, they are paid :( https://www.npmjs.com/#

@warpech
Copy link
Contributor

warpech commented Mar 10, 2017

"Organizations" is a paid plan (https://www.npmjs.com/#) that adds two features:

  • Publish and download private packages
  • Manage permissions for groups and teams

I think that we don't have need for any of the above.

Could you just add me and @tomalec as co-owners? https://docs.npmjs.com/cli/owner

@alshakero
Copy link
Collaborator Author

@warpech on it.

@alshakero
Copy link
Collaborator Author

alshakero commented Mar 10, 2017

@warpech turns out adding you as collaborators is equivilent to this. So you're owners already.

image

@alshakero
Copy link
Collaborator Author

@warpech I think this is good to close?

@alshakero
Copy link
Collaborator Author

Closing as all done.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants