Firebase generator#643
Conversation
|
Great! Add Rob as the reviewer as he's the MasterOfGenerators™ |
The Yoda condition, thou shall not use
…into firebase_generator
peterp
left a comment
There was a problem hiding this comment.
It looks good, I left some ideas about improving the data-structure for the imports.
| const importStatements = imports.map((imp) => { | ||
| return `import ${imp.import} from '${imp.from}'` | ||
| const importWithFrom = imp.import === null ? '' : `${imp.import} from` | ||
| return `import ${importWithFrom} '${imp.from}'` | ||
| }) |
There was a problem hiding this comment.
If my suggestion about changing the imports data-structure works out, then you could remove these lines.
cannikin
left a comment
There was a problem hiding this comment.
You'll need a way to write out the custom auth.firebase template instead of the single auth template we have now. You could do a check that if the provider has its own template, write it out, otherwise use the generic auth.js one we already have.
…ailable providers ), fixed implementation of imports.
I've sent a fix, unfortunately I have issues testing it ( the template file appears when I use the rwt kit, but when I start the generate command for some reason it disappears and therefore, cannot be found ). I'll have to leave testing to one of you I'm afraid :-, this is out of my reach. |
I've experienced it too, I usually run Sometimes the rwt command blows up. |
|
I can definitely test it though! |
|
ah thanks @peterp that'd be nice! Good to know it's not an isolated issue ( isolated in front of my computer screen x) ). Both the copy and copy:watch have proved to be unreliable, I really think it was an awesome way to get people to contribute so I hope it'll be fixed soon enough :). Either way, brutally setting up symlinks can do a dirty trick to make do, but I'm happy to trade it for your solution. |
There was a problem hiding this comment.
I love all these changes, the only thing I'd ask is to keep the template with just a .template extension. Maybe make it firebase.auth.js.template or auth.firebase.js.template. We've been keeping the .js.template extension consistent among all the generators.
|
@cannikin no problem, makes sense :) sent the fixes for review |
|
Awesome! And you've tested both the firebase generator and at least one of the others to make sure everything still runs smooth? |
@cannikin I have tested auth0, still runs smooth ;), unfortunately I met issues while testing firebase:
@peterp offered help with testing :) I'd say the time has come :) ( or we... test on prod.... hush hush.... x) ) |
|
Hmmmm...that's very strange...I don't see why it would be treated any different than any other template we have, other than the fact that the filename has 4 parts instead of 3... |
|
I don't see either. For the sake of it I tried removing '.auth' to keep 'firebase.js.template' --> changed zits, it still gets removed while using the generate command.. Can you try it on your side? |
|
@noire-munich The generator worked for me! One thing I noticed is the indentation is off in the code that's added to export const config = {
imports: [
`import * as firebase from 'firebase/app'`,
`import 'firebase/auth'`,
],
init: `const config = {
apiKey: process.env.FIREBASE_API_KEY,
authDomain: process.env.FIREBASE_AUTH_DOMAIN,
databaseURL: process.env.FIREBASE_DATABASE_URL,
projectId: process.env.FIREBASE_PROJECT_ID,
storageBucket: process.env.FIREBASE_STORAGE_BUCKET,
messagingSenderId: process.env.FIREBASE_MESSAGING_SENDER_ID,
appId: process.env.FIREBASE_APP_ID,
}
const firebaseClient = ((config) => {
firebase.initializeApp(config)
return firebase
})(config)`,
authProvider: { client: 'firebaseClient', type: 'firebase' },
}Which looks ugly here, but it generates the code correctly! Also can you rename the |
|
@cannikin hooray! |
Resolves #642