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

🎉 2.0.0 New Feature that can use on React-Three-Fiber, Javascript & even Typescript as well! #3

Merged
merged 44 commits into from
Nov 18, 2023

Conversation

ammein
Copy link
Collaborator

@ammein ammein commented Nov 17, 2023

I made a monorepo package on easy to just publish one npm package but can import different usage types such as React, Javascript and Typescript as well! This can help all react-three-fiber coders use GlslPipeline immediately!!!!

CHANGELOG:

  • Added typescript supported to the package
  • Added Preconstruct cli on easy manage package and test the examples file live without using npm link.
  • Added manypkg that can check monorepo status.
  • Added directory symlink to README.md & LICENSE.md due to that files located on inner directories.
  • Added copywriting for README.md file for easy instructions on what to do when install the package.
  • Added changesets/cli on easy publish monorepo package by simply run simple commands. Please check CONTRIBUTING.md file on how does the package & publishing works.
  • Added new examples packages to test either react or vanilla packages (TODO: Add more Typescript examples)
  • Added Babel config for build the package before publishing the package to npmjs.
  • Added TSConfig for Typescript linting config.
  • Added Github Actions to check on the package validation & also added FUNDING.yml to have sponsor button available on this github repo.

Please refer to /package/CONTRIBUTING.md file to know about this new monorepo style & how is it going to publish. You can reach me anytime to work on this together. 😁

ammein added 30 commits November 8, 2023 20:05
…nd inside 'examples' folder. Then install all dependencies including 'glsl-pipeline'. Don't worry, it is actually grab local package as workspaces testing. No need to do 'npm-link'
ipt and dispose method
…act. Also add another parent memo for GlslPipelineReact on 'props' issue re-render on 'useMemo'. Also adjust typescript to have THREE.ShaderMaterial attached on react so that user can get lint on what to apply options on GlslPipelineReact.
@patriciogonzalezvivo
Copy link
Owner

Oh wow! Thank you so much @ammein ! this is wonderful. This means once pushed to npm regular JS + threeJs projects will still be supported in the same way? Excuse my ignorance respect to Web development

@ammein
Copy link
Collaborator Author

ammein commented Nov 18, 2023

Absolutely! No breaking changes at all! They can use manually as they used to from V1 release. Only additional method in GlslPipeline class called:

  • New Shader Material Options on GlslPipeline constructor new GlslPipeline(renderer, uniforms, options) so that developer can pass on Shader Material options such as wireframe:true to enable wireframe shader view on canvas. For uniforms and options argument, I maintain on keeping them optional argument.
  • dispose() To dispose all texture, renderTargets & material useful in react environments.
  • branchMaterial(name) can add either single string or an array of string. Useful for multiple defined conditions.

I already created changeset initializer using command changeset init. So you will release the package using changeset/cli commands:

# Build the package first
yarn build

# For contributor, run
yarn changeset

# For maintainers release package
yarn changeset version

# For maintainers, contributors and owners to release package
yarn changeset publish

Side Note: There are some grammars and docs that I need to fix. Later i will push the last code to fix all copywriting and adjust types-helper code.

Aren't you excited?! I myself can't wait to use this on my project! 🥳

You can explore more on my forked repo for this pull request:
glsl-pipeline-react

@patriciogonzalezvivo
Copy link
Owner

Yes I am! This is truly wonderful! Thank you so much. This will allow to use glsl-pipeline in so many project! You really have taken things project to the next level. Incredible work architecting the multiple release and examples too.

@@ -0,0 +1,7 @@
The MIT License (MIT) Copyright (c) 2023 Amin Shazrin

Choose a reason for hiding this comment

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

let's keep the original license as is:

The MIT License (MIT) Copyright (c) 2023 Patricio Gonzalez Vivo

....

LICENSE.md Outdated

Choose a reason for hiding this comment

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

Based on the behavior on https://github.com/ammein/glsl-pipeline-react seams Github don't automatically picks up the LICENSE.md when is a symbolic link. Let's leave a copy on the root folder just for visibility

find the full documentation for it [in our repository](https://github.com/changesets/changesets)

We have a quick list of common questions to get you started engaging with this project in
[our documentation](https://github.com/changesets/changesets/blob/main/docs/common-questions.md)

Choose a reason for hiding this comment

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

Nice addition! so this is used through yarn to build the packages for JS, TS and React. Right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one is automatically generated by using command yarn changeset. Leave it as it is so that changeset can find this folder with its auto-generated README.md inside .changeset folder.

Choose a reason for hiding this comment

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

nice!

- name: Install and Build 🔨
run: |
yarn
yarn ci

Choose a reason for hiding this comment

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

Thanks for adding CI to it!

Copy link
Collaborator Author

@ammein ammein Nov 18, 2023

Choose a reason for hiding this comment

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

No Problem! It looks similar to npm run ci! This script will validate & check the packages, and finally test build the package to see if it is a successful build or not when pushed to the main branch Git Hub.

Choose a reason for hiding this comment

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

nice!

"vite": "^4.4.5"
},
"dependencies": {
"@react-three/fiber": "^8.15.11",

Choose a reason for hiding this comment

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

Is react-three/fiber required here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good one! I ran the script from the '/scripts/updateAll.sh' file that runs every /examples/** folder to add/update packages needed for each examples. Maybe I need to tweak the bash file to run specific package folder on each framework examples.

Choose a reason for hiding this comment

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

got it

"version": "1.0.6",
"description": "Prototype complex pipelines directly from a single shader by branching it into stages.",
"name": "glsl-pipeline-monorepo",
"private": true,
Copy link
Owner

@patriciogonzalezvivo patriciogonzalezvivo Nov 18, 2023

Choose a reason for hiding this comment

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

Oh, I get it, at this level the package is private and contain the other 3 packates (vanilla JS, TS and React). That's why is a monorepo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely! The root folder wouldn't publish to npm package due to its private package. Only public packages & allow public access to be publish on npmjs. Simply run the command as I suggested 😁

Choose a reason for hiding this comment

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

fantastic

"main": "index.js",
"license": "MIT",
"author": "Patricio Gonzalez Vivo <patriciogonzalezvivo@gmail.com>",
"maintainers": [
"Amin Shazrin (https://github.com/ammein)"

Choose a reason for hiding this comment

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

add me and @marioecg ( https://github.com/marioecg ) here please. Mario helped a big deal on the first version.

],
"devDependencies": {
"vite": "^4.4.9",
"resolve-lygia": "^1.0.4"

Choose a reason for hiding this comment

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

If I understand correctly resolve-lygia is not really necessary here. Right?

```js
import { GlslPipeline } from 'glsl-pipeline';
import { GlslPipelineReact } from 'glsl-pipeline/r3f';
import { GlslPipelineClass } from 'glsl-pipeline/types';

Choose a reason for hiding this comment

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

types stands for TypeScript? right?

u_speed: { value: 0.5 },
...
},{
side: THREE.DoubleSide,

Choose a reason for hiding this comment

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

nice addition

✅ "dev-vanilla": "preconstruct dev && yarn workspace <package-name> dev",
}
```

Copy link
Owner

@patriciogonzalezvivo patriciogonzalezvivo Nov 18, 2023

Choose a reason for hiding this comment

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

Let's add here:

## Acknowledgements 
Special thanks to main contributors: 
* [Amin Shazrin](https://github.com/ammein/) 
* [Mario Carrillo](https://github.com/marioecg)
* [Matt Henderson](https://github.com/matthen)

@patriciogonzalezvivo
Copy link
Owner

patriciogonzalezvivo commented Nov 18, 2023

@ammein I took a look to the code. Fantastic job! I get it now (or at least I think I do, hehehe) this is massive upgrade!
I love the fact that you manage to bring support for both TypeScript and React without sacrificing support for vanilla JS for dinosaur like me : ) . Thank you! This will make things better for a lot of people.
I added a couple of comments, some propose some minor things. Once that's in I can merge, and then I will follow through your instructions on yarn to publish a new npm version. I might have questions at that point.
Again thanks you!

@ammein
Copy link
Collaborator Author

ammein commented Nov 18, 2023

All done! You can merge safely now @patriciogonzalezvivo 🥳

Changelog:

  • All reviews for changes have been made.
  • Update docs on grammar and add the Troubleshooting section in CONTRIBUTING.md.
  • Update glsl-pipeline-react.tsx on useEffect that causes infinite loop re-render. Remove some dependency arrays that are causing the re-render and memory leaks. Now it is stable ❤️
  • Add a new typescript example for GlslPipeline options. View and test the example package in examples/react-ts/typescript_shader_options.

@patriciogonzalezvivo
Copy link
Owner

Fantastic, let's do it <3
Thank you!

@patriciogonzalezvivo patriciogonzalezvivo merged commit 3ceae83 into patriciogonzalezvivo:main Nov 18, 2023
@patriciogonzalezvivo
Copy link
Owner

@ammein yarn changeset doesn't allow me to do glsl-pipeline, what am I doing wrong?
image

@ammein
Copy link
Collaborator Author

ammein commented Nov 18, 2023

Ahh, I already did glsl-pipeline for you firsthand. You can just simply do yarn changeset publish for now. In future, if you want to publish again, you will need to do the yarn changeset first 👍🏻😁 Sorry for helping firsthand that confuses you @patriciogonzalezvivo

@patriciogonzalezvivo
Copy link
Owner

Awesome! Just publish works like a charm. Thank you so much!

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.

2 participants