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

fix : publish action #3

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

shivam-Purohit
Copy link
Contributor

solves issue #2

Converted the action to support node instead of composite action. I also draft published and used it in my repo for testing and works fine now which was not working in composite, I think the workflow does not support composite type on publish but i am not sure.

@mlodic
Copy link
Member

mlodic commented Oct 11, 2023

hey, why did you embed all the node library code here? this can not be the way to use a library

@shivam-Purohit
Copy link
Contributor Author

I think it is supposed to run on the browser itself. Also we are not adding any pre-installations of any package like twitter-api-v2. It is directly fetching from the node-modules.

@mlodic
Copy link
Member

mlodic commented Oct 12, 2023

I am talking about the 300+ files you added and the folder twitter-api-v2. I don't get why it is needed there.

@shivam-Purohit
Copy link
Contributor Author

Yes, I am talking about the same. These are part of the node_module folder and come under the twitter-api-v2. When we run a nodejs application it requires certain libraries like in our case twitter-api-v2. Normally when we run the app locally we run the npm install command to get all the required modules and are stored in node_modules folder. So node_module is necessary for a app/script to run in node environment.
I intentionally pushed the node_modules because:

As we are not running this script index.js in our local environment and we are running it on the browser itself, we will need them in the repository for running the script. Unless that it will throw an error called module not found.
As we publish this action the action will redirect to the index.js and create node environment but that does not include the libraries we require. Unlike python normally installing a package won't also work as it will always look under the node_modules.

You can refer to this documentation about creating a javascript-action. There are some alternative to this but as we won't be changing our code that much I think it is unnecessary.

The action that we were using before does the same with a lot more modules than us.

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