Convert to NPM package #37
Conversation
|
@alyssaxuu let me know if you've got any feedback on this or if it can be merged. Next, I plan to explore integrating with React |
|
@alyssaxuu this is ready to merge to make this an NPM package with a simple build process that generates the demo code. You just need to decide on a package name that is available on npmjs.com. Any feedback to get this merged? |
|
@joelvh you did some good work here. 👏 |
|
@alyssaxuu wanted to see if you have any feedback or thoughts on this PR and #38 and #39 |
|
@joelvh I appreciate the effort - although I personally would prefer to keep the library simple here in GitHub in terms of files & code. I also did some bug-fixing/improvements on my code so that would be lacking in this pull request I believe. Maybe if there was a way to keep a "minimal"/compressed version of the code together with yours as to be able to convert to a NPM package would work. But yeah, the simplicity and conciseness of this library is important to me. |
|
@alyssaxuu updates can be merged in, and .gitignore can be updated to include the generated output in the git repository if you like. |
Hmm okay, so then there would be the original "vanilla" version of the library with the original demo (separate folder or whatever) & this version which works with NPM? |
|
Yeah, this is the vanilla code in this branch: https://github.com/joelvh/flowy/tree/master/src/demo The NPM package makes it possible to install the code along with other packages and reference it as described here: https://github.com/joelvh/flowy#installation The NPM package also allows you to have some simple development tools available when working on the source code. You can see that there is a command that will launch the demo in your browser, and it will also live-reload that browser code if you edit the source files (e.g. when working on features or bug fixes): https://github.com/joelvh/flowy#demo As that last link indicates, you can just build the compiled/compressed files and not run it in the browser -- the |
|
@alyssaxuu P.S. I'm happy to help get the latest updates merged into here. |
Right, so you're suggesting different branches for each version in that case (also for the React one)? |
|
@alyssaxuu no, once these changes are merged in, it's all on the main branch. I was just saying that those links I put in my comments above are to my branch with the changes in this PR so you can see what it looks like before merging. The "Files changed" tab in this PR doesn't really show where the files may have moved to, so linking to my branch just helps clarify things (hopefully). |
Ooooh okay, got it yeah. I see, seems fair enough to me. So would just have to add the latest changes & bug fixes to this and that would be it I suppose. Although the NPM part would still have to be set up I assume (and also picking the name since it can't be just "flowy"). |
|
@alyssaxuu yeah, it's just bringing those updates in. I checked, and "flowy.js" is available, if you want to use that. You could rename the repo as "flowy.js" too, but you don't have to. I'll take a look at getting your latest changes into this PR. |
|
@alyssaxuu I've merged in your changes and also updated .gitignore to commit both the /demo and the /dist folders into the repo. The /demo folder has the result of building the demo source code into a viewable demo using the command The /dist folder has the result of building the .js and .css files into usable files in various js module formats for use on the CDN or importing into packages when downloading this project without NPM. You can see both folders in my branch from this PR: https://github.com/joelvh/flowy |
Got it, perfect! Re: name, flowy.js is fair enough. Is it ready to be merged already? |
|
@alyssaxuu looks like the conflicts are resolved, so you can merge this if you're happy with it. |
I'm still adding some new things and currently it's easier for me to do so as it is. I've been thinking about adding a method to remove individual blocks and a few other enhancements to make the library more usable, as there's been high request for it, today I've also added a new method to allow the user to decide if rearranged blocks dropped in the canvas are deleted or re-attached to their previous parent. So I think before merging & turning it into a npm package I might want to make sure that there's some more new functionality. On a side note, in the branch you shared a few comments above, I notice .editorconfig & .prettierrc - would those also be in the merge? |
|
Makes sense that you have a lot of improvements you'd like to make. I think you can merge this and not worry about publishing as an NPM package quite yet. Merging this branch will make the project be setup structurally for NPM, and your workflow won't really need to change. Just so you know, the changes in here are simply moving and renaming the files you've continued to make updates and improvements to in your branch. To illustrate, here's how the files were moved, but still exactly the same as what you have. Then to rebuild the minified files you run: Then to rebuild the demo files you run: To answer your questions about If you don't want that, it can be removed. But it helps make sure everyone's Javascript coding style is the same when making changes. In the meantime, I'll merge your changes back in, but again, I just moved the files to a subfolder, so you can keep working there after the merge and don't need to do anything about publishing an NPM package yet if you don't want to. |
|
@alyssaxuu one request I have - could you please add descriptions in your commits that explain what you changed or why? It's hard to make sure I'm pulling all the latest changes into this branch when they all have the same or similar commit messages. Thank you! |
Ahh okay, I swear I should have looked at it more closely haha, now it all makes sense. That works I guess then. As per the files, I'm not really sure if they would have to be kept in. I have a few enhancements in mind for the library, but I don't really plan on turning it into anything much bigger, so I don't really see a need personally to have that for PRs. I do have to ask about the minified versions of the file. Currently I have jsDelivr as an option that pulls from flowy.min.js and flowy.min.css, for people who want to implement the library that way. In your repo I'm not sure what those files would be? Is it flowy.js and flowy.css in the dist folder?
Yes, that's a good point, I'll be doing that. |
|
I've been thinking of using this library and having it as an npm package would really help me! @alyssaxuu this library is amazing! @joelvh thanks a lot for the effort.. this is great work! Really looking forward to the PR being merged! 💯 |
|
@alyssaxuu I just merged in your latest changes in this branch. Regarding the files to put on your CDN (jsDelivr), yes, its the files in the There are multiple formats that would allow users to load it as regular js files in a webpage, or as a commonjs package, and so on. I'd recommend uploading all of them to jsDelivr so people have options for which they need. As for the files like Can you merge this today? |
Perfect! So flowy.js.js would be flowy.min.js and flowy.css would be flowy.min.css as I understand it? And sure, I can merge it today :) I just want to make sure it's all good haha. |
|
@alyssaxuu yup! And, great! |
|
All right, it's done. So I guess all that's left now is create a NPM package. I wonder if it's best for me to try adding some new methods before doing so? Or w/ versioning it shouldn't be a big deal? |
|
@alyssaxuu woohoo! 🥳 What I'll do is I'll create the "flowy.js" NPM package and add you as a collaborator. Just let me know your npmjs.org username. By the way, would you be open to hopping on a call or screenshare? I can walk you though a couple things so you feel comfortable with the new setup (which isn't a huge change, but might give you more ideas too). |
My username is "alyssaxuu". Also, I feel like the ReadMe is confusing, now that I've taken another look. There should be two ways to implement Flowy, 1) Through NPM, and 2) Using JsDelivr / self host. I see in the latter you put "Run npm run build:web", I assume this was meant for #1? And appreciate it, unfortunately I am quite busy ^^ I'll just look into it to get more familiarized. |
|
Just saw the package - I'm confused, shouldn't it be called flowy.js instead of "@flowy.js/flowy"? |
|
@alyssaxuu I've created issue #54 to discuss things to clean up and fix. Regarding the name, unfortunately NPM didn't allow The benefit of that scope/namespacing is if you have multiple packages, you can put them all in there. Flowy could theoretically have Could you add more "tasks" by editing the description in #54 and make any more comments there regarding your thoughts from this thread? I figured that would be an easier place to track since this PR is now closed. |
|
I can't see the changes in |
|
@joelvh if you get a fork going I'm happy to work with you on that. I want to use this idea in a react project I have. |
Yeah I reverted the changes, I am working on something (optimization / simplicity side of things) so I'm keeping as it was originally for now. |
|
@alyssaxuu was there something wrong with what was merged? |
|
@dardanos I have a fork that I've made the pull requests from. I'm not sure what was wrong with this pull request. I also created Issue #54 to work on any additional improvements, but looks like that was deleted. @alyssaxuu let me know what the issue was. maybe based on what you described about not wanting contributions means I should just work on my fork? |
There was no issue, I've just been trying to optimize / simplify the library as well as deal with some methods, basically before taking the step to turn this into a NPM package I would like to make sure the library is at its best (adding several features / enhancements I've been getting both here & through email), so for the time being I will keep it as is. Like I mentioned I don't really have plans to expand the library so I would like to ensure as much as possible that I don't have to keep on fixing bugs and adding new enhancements consistently. |
|
@alyssaxuu I'm happy to help contribute and maintain the library for/with you, so you don't have to feel like it's all on you to make improvements. It seems like there are several people investing in using Flowy in their projects, so they are incentivized to make things better and help out. If you don't want people to contribute or maintain this for you, I'm happy to make improvements on my fork. As you can see from the other pull requests, i think there are improvements that can also be made to use ES6 for more modern coding styles, and React integration examples. |
I don't mind making improvements myself, it's not a matter of it being tedious - I just think rolling them out inconsistently makes it hard for people who want to use this library to stay up to date, so I just want to make sure it's polished and with all the enhancements and methods before I make any new releases. I appreciate contributions to the library (as I've managed to fix some bugs for example early on thanks to those), but I'm happy keeping it small, simple, and minimal (as it is described). I'm just going to be setting a clear roadmap for the improvements, as well as I talk with people who use the library to understand the needs better to make a proper release. |
|
@joelvh I think we can wrap the work @alyssaxuu did in a Lerna project as the "engine", provide storybooks, and a react wrapper. It will be tedious to maintain the engine initially, but maybe a lot of people are interested in contributing to a react library and it ends up deviating from what it is today. @alyssaxuu, will you be ok if we work on @joelvh's fork so that we can make progress? If the answer is yes, we just need to make sure we don't use the name flowy in the package, and we give credits to this project. |
Feel free to work on this fork & make your own version if you want - but like I said I am eventually going to turn it into a NPM package & have support for React and other frameworks, I'm just working on several enhancements: #53, #51, #50 (maybe), ~#29 (I want to handle logic / conditional blocks in a better way, so I want to think of a good approach), and aside from those I am also working on a function to delete blocks by ID, with the option to include all the children when being deleted, or just an individual block (if it has children, the children would be rearranged with the parent of the deleted block). I am also exploring the possibility to handle multiple trees as per popular request. I just would like to make all those changes and enhancements + optimize & allow for creating a NPM package at the same time in a single release. Otherwise it would be confusing to roll things out progressively with the library changing constantly. |
|
For what it's worth @alyssaxuu the nature of any project like this is that there will be multiple versions of it. If you think about everyone who already has started using Flowy, every time you make an update, they need to decide if they will upgrade to the latest or not. Of course, the goal is for everyone to upgrade to the latest. One thing that is currently hard to do is know what "version" is the latest. Updating the "version" property in package.json, adding a git tag with the version, and adding a GitHub "release" with the version and a list of updates, is what makes using a package like Flowy much more manageable. Developers using Flowy are likely using many other frameworks as well, and need to keep up to date. Having Flowy use package.json and NPM only makes this easier for developers. Setting the project up like that, developers will know immediately when updating their project dependencies/packages when Flowy is outdated -- rather than having to check this GitHub page manually on a regular basis. Detailed commit messages also help developers understand what changed and why. All this to say, it's great that you want to add more features before you release an NPM package, and that you're open to React/framework integrations. However, at the moment, for me and presumably other developers like @dardanos, working with Flowy is harder because it is extra work to know what the status of the framework is without some simple standards like versioning. Additionally, allowing others to submit PRs and take work off of your hands makes things more manageable for you. Flowy can remain simple and focused -- things that go beyond what you intend for the engine can be published in separate packages to build on the engine. I would very much like to create and maintain those, ideally in the flowy.js NPM org. Lastly, as I said, releasing to NPM now (rather than waiting) is not publishing an "incomplete" package because developers are already using Flowy and might want to have a better experience keeping track of "how far behind" their copy of Flowy is from your latest updates. If you saw the version I had in package.json, it was version 0.1.0 -- that was intentionally not 1.0.0 because you are currently working toward what sounds like your "complete" 1.0 release. A version starting with 0.x is easily identifiable as "a work in progress" and lets developers know that by the time version 1.0 is ready things may change. But at least they have something to go on to know what the status of Flowy is. (Check out how most other open source projects use versioning here.) I am personally stuck right now trying to build on Flowy, make incremental improvements and not diverge from Flowy because my changes (which keep everyone in mind, not just me) are not being merged in, I don't know if anything I submit in the future will be considered to be merged in, and working off of a fork for a long period of time begs the question whether I (and others) will ever be able to pick up speed in making Flowy better and better with/for you. So, I am purely advocating for allowing us to help you do things faster, while keeping all other developers using Flowy in mind. I'd like to be able to contribute (and help maintain) this project as I have helped with many other open source projects. I've said my piece and hope that I can use Flowy as you're developing it, but in the meantime will maintain my fork and pull in the latest as you make updates. |
Yes, I understand that, and it is due to the versioning problem that I want to stop and add all the new changes that will "drastically" change the library (at least in terms of structure) at once. I know if I release a NPM package I will be able to update it with the new changes, I just simply want to take that step after making the improvements, that is all. It will just be easier for me to announce all the changes in bulk, as a "Flowy v2", and try to ensure that people will be satisfied with the library in that state. That's just how I personally see it. Like I said I will stop pushing new commits (with the exception of some bugs I have recently fixed which I deemed critical due to the hectic updates I made the past weeks adding a new method & other little enhancements, which in retrospect could have waited) until I have this ready, so you shouldn't worry about "staying up to date". That's basically what's currently happening, I'm just sort of "overhauling" the library to make it more useful for everyone. |
|
OK, sounds good @alyssaxuu. On the subject of "overhauling", are you open to using more modern ES6 syntax? My other PR (a different branch from the NPM branch) was intended to introduce JS syntax that is used more and more, which uses features such as "destructuring" as you can see here: https://github.com/joelvh/flowy/blob/feature/react/src/engine/index.js#L15 It also uses Curious if you're open to that as well? |
|
@dardanos for the time being I've published the NPM package to https://www.npmjs.com/package/flowy-engine - that may change to use the official NPM package when ready. |
|
I like the idea of npm. Making updating and building much easier. And also if versions are named correctly (with major, minor, and patch) then there won't be an issue with breaking changes. For example the current version can be 1.x.x and then since we are changing the API we will release it as 2.x.x. The latest version will always be on master branch while old versions and experiments can be branched out to have their own individual branches. Semver: Given a version number MAJOR.MINOR.PATCH, increment the: MAJOR version when you make incompatible API changes, |
@alyssaxuu note that you will need to choose a package name because "flowy" is already taken
Credit goes to #12 for the UMD build process.