feat: building google analytics plugin #37
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
@fmvilas Please kindly check out this draft PR and help me out 🙏 |
src/lib/client-events.js
Outdated
| clientEvents.on = eventEmitter.on.bind(eventEmitter); | ||
| clientEvents.once = eventEmitter.once.bind(eventEmitter); | ||
| clientEvents.emit = (eventName, eventPayload, ...args) => { | ||
| eventEmitter.emit(eventName, eventPayload, ...args); | ||
| }; |
There was a problem hiding this comment.
I wonder if we can just do this:
| clientEvents.on = eventEmitter.on.bind(eventEmitter); | |
| clientEvents.once = eventEmitter.once.bind(eventEmitter); | |
| clientEvents.emit = (eventName, eventPayload, ...args) => { | |
| eventEmitter.emit(eventName, eventPayload, ...args); | |
| }; | |
| module.exports = eventEmitter; |
yarn-error.log
Outdated
| @@ -0,0 +1,112 @@ | |||
| Arguments: | |||
There was a problem hiding this comment.
I think you can get rid of this file 😄 Also, add it to .gitignore so it doesn't happen again.
| const googleAnalytics = module.exports; | ||
|
|
||
| googleAnalytics.init = (window) => { | ||
| ReactGA.initialize('UA-000000-01'); |
There was a problem hiding this comment.
I wonder how can we make this id configurable without exposing server-side secrets into the client code 🤔
There was a problem hiding this comment.
What if the id can be passed as an extra arg when emitting the event on page render. @fmvilas what do you think?
Hi @fmvilas. I'll do that |
|
Kudos, SonarCloud Quality Gate passed!
|
|
@AceTheCreator I copy your comment from issue :)
The problem is that you don't have any triggers in the client side due to fact that writing code in |
@magicmatatjahu what do you think will be the best approach in tackling this problem? |
| import React from 'react'; | ||
| import googleAnalytics from '@asyncapistudio/plugin-google-analytics'; | ||
| import AppContext from '../contexts/AppContext' | ||
| import '../css/tailwind.css' | ||
| import clientEvents from '../lib/client-events'; | ||
|
|
||
| class AsyncApiStudio extends App { | ||
| static async getInitialProps({ ctx }) { | ||
| if (!ctx || !ctx.req) return {} | ||
| function AsyncApiStudio({ Component, pageProps, context }) { | ||
| if (typeof window !== 'undefined') { | ||
| clientEvents.on('page:render', () => googleAnalytics.init(window)); | ||
| clientEvents.emit('page:render'); | ||
| } | ||
| return ( | ||
| <AppContext.Provider value={context || {}}> | ||
| <Component {...pageProps} /> | ||
| </AppContext.Provider> | ||
| ); | ||
| } |
There was a problem hiding this comment.
@AceTheCreator I will answer here for you question.
As I wrote, the code:
clientEvents.on('page:render', () => googleAnalytics.init(window));
clientEvents.emit('page:render');will be only executed on the client side and you will lost all events binded to clientEvents registry. As you wrote in previous comments, googleAnalytics.init only works when you bind this plugin as clientEvents.on('page:render', () => googleAnalytics.init(window)); in this file. So I see steps to do:
-
load events from plugins to
clientEventsregistry. We must remember that they should be loaded on client side, so loading in this file is probably the best place, but:-
to load events we use the file system. It doesn't exist on client side, so we must handle this problem on different way, I mean load events from external packages/plugins but without filesystem. We can use https://nextjs.org/docs/advanced-features/dynamic-import#with-no-ssr to load dynamic modules on client side :) However there is an another problem, because we must know about names of external packages/plugins. So this is the hardest part. How to get the packages name, and then load events from them? If you don't understand, please see example:
If we will use the dynamic import from nextJS, we will have something like:
const DynamicPlugin = dynamic( () => import('@asyncapistudio/plugin-google-analytics'), // I didn't test it, so I don't know if this works, but should { ssr: false } ) clientEvents.on('page:render', () => DynamicPlugin.init(window)); ...
So we have hardcoded name of the plugin in source file. How to get external plugins from
config.pluginsarray? To be honest I don't know, because then we must to do:const pluginsName = ... // here we must load the names of plugins from `config.plugins` array, but we cannot use filesystem, because data from it will be lost in client side, maybe some logic in `getInitialProps` will help for (const pluginName of pluginsName) { const DynamicPlugin = dynamic( () => import(pluginName), // I didn't test it, so I don't know if this works, but should { ssr: false } ); clientEvents.on('page:render', () => ... bind page:render event from plugin); }
-
-
Execute
page:renderevent.
Let me know if you understand the problem. I really don't know how to solve it the easiest way, because we're trying to bind the filesystem to the client side which is hard to do, I can say it is impossible. I don't see even very complicated solution at the moment for this problem 😢
I will try to spend some time on this problem, but I don't give guarantee that I will be able to solve something.
There was a problem hiding this comment.
@magicmatatjahu sorry for the late response 🙏 I was quite busy clearing all blockers that will interfere with GSOC.
I get your approach to solving this problem and I'll try working on it to see what I can come up with. Look out for a draft PR and good job man 👍
There was a problem hiding this comment.
@AceTheCreator Hi! No worries, we know that you also study :) It's an open source so we also know that people work in the free time :) Please also don't waste your time on this issue, because it's really hard to make it in the good way. You should focus on your GSOC project :)
|
This pull request has been automatically marked as stale because it has not had recent activity 😴 |
|
@AceTheCreator I opened issue #80 Probably we will remove the NextJS and go with pure React app, so I will close PR, or do you have a different opinion on the subject? |
@magicmatatjahu It's fine. So will any suggested plugins be built separately? |
|
@AceTheCreator The Studio will be SPA application, so there will be plugins - you can read proposal here. If you mean plugins for (server) API then we can also reuse my proposal - you can add comment/ask about plugins in the #86 issue, but to be more specific - the plugins will probably be written in monorepo and not as separate npm packages - but that can always be changed. |
Description
Related issue(s)