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

added transpiled lib generation with prepare script (fixes #65) #72

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dlong500
Copy link

This completely fixes the remaining issue in #65 and also allows developers using bundlers to take advantage of access to non-minified code (see details below).

  • Transpile module and output to lib folder with prepare script (lib folder is not committed, as the code gets generated when package is published to NPM (or directly installed via pointing to github).
  • Changed main field in package.json to point to lib/index.js so bundlers like webpack can access full transpiled code (not minified).
  • Also updated UMD build in dist folder so the UMD build is current with latest source.
  • Note that the babel config for directly transpiling needed to be adjusted, so I added webpack-specific babel configuration within the webpack config files to keep the UMD build correct.

…d to point there, also updated umd build in dist
@louisrli
Copy link

louisrli commented Dec 29, 2020

Does this need to be imported in a certain way to resolve #65? I've installed it via Github (through yarn) but I'm still running into the window error.

@dlong500
Copy link
Author

Does this need to be imported in a certain way to resolve #65? I've installed it via Github (through yarn) but I'm still running into the window error.

Are you sure you installed the code directly from this pull request? This PR hasn't been merged into the main project, so unless you are specifically using the code in this PR you won't see my changes. I've never actually tried using yarn to install from a specific pull request so I'm not sure how that would work.

Regarding how to import the module, my fixes only deal with transpiling and build artifacts--no actual module code was changed. The examples in the README should work fine. It will work with a bundler like webpack using an import/require or you can use the UMD build in dist without any bundling.

I'm hoping @zsajjad will have time to review and merge this PR soon.

@dlong500
Copy link
Author

@zsajjad You've probably been tied up with other things, but can you take a look at this PR? It would be nice to get a new release cut.

@dlong500
Copy link
Author

@zsajjad Pinging you again to see if we can please get this PR merged sometime soon.

@chuma9615
Copy link

@zsajjad I'll really love to see this feature implemented! Please merge if you have time.

@bephrem1
Copy link

Looking for this solution, would appreciate a merge & publish

@dylman123
Copy link

This issue also affected me. Hopefully @zsajjad will merge soon :)

@danwetherald
Copy link

Its a shame this is not simply merged in, what is the hold up?

@WBialekMerixStudio
Copy link

What is the status of this PR?

@dlong500
Copy link
Author

@zsajjad I'd really like to get this PR merged as there are clearly a number of people that are waiting. I'm sure you have a lot going on but it does appear you still have activity on a weekly basis with various Github projects. If we could get this one PR merged that would prevent having to fork the project (and I don't really want to be maintaining a fork). Can you let us know your plans for this library moving forward?

@danwetherald
Copy link

If @zsajjad is not planning on maintaining this anymore, I am happy to fork if need be as I have already merged this into our fork.

@tmbtech
Copy link

tmbtech commented Feb 8, 2022

If @zsajjad is not planning on maintaining this anymore, I am happy to fork if need be as I have already merged this into our fork.

Hey @dan003400 curious if you decided to fork the repo - I was thinking about doing the same or maybe we can work together and create a new version

@danwetherald
Copy link

Hey @tmbtech - yes we have forked the repo here and it will remain to be maintained.

https://github.com/bettercart/react-facebook-pixel

Let me know if you need anything merged.

@tmbtech
Copy link

tmbtech commented Feb 8, 2022

Awesome! thank you!

@dlong500
Copy link
Author

dlong500 commented Feb 8, 2022

@dan003400 I'm a bit confused. In looking at the source on your fork I see other changes but I don't see this particular PR merged. The main field still appears to be pointing to the UMD minified build. Maybe you worked around the issues I was having some other way?

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.

8 participants