image-charts / mjml-chart

<mj-chart/> for MJML framework
https://image-charts.com
78 stars 5 forks source link

Update mjml-chart to use mjml 4 #1

Closed jsjoeio closed 6 years ago

jsjoeio commented 6 years ago

Hello @FGRibreau ! I'm working on migrating mjml-chart to use mjml 4. I was hoping it would be pretty easy but I'm stuck at the moment:

Here’s a log of everything I've done so far to migrate mjml-chart to mjml 4.

  1. Fork and clone repo.

  2. Install dependences with npm i - no issues (as long as you're using node v8.10.0)

  3. Tested everything by running npm test - all tests pass

  4. Update mjml-core and mjml-image to v4.0.5. Then run npm test

    • This produces an error stating that: (TypeError: (0 , _mjmlCore.MJMLElement) is not a function So next thing I did was look at mjml-core to see what changed. Based on how they create components in MJML v4 (here's an example, it looks like they use a class called 'BodyComponent' instead of 'MJMLElement'.
  5. Edit scripts/templates/component.js.tmpl and lib/index.js to use 'BodyComponent'

    • Here are the lines I changed:
      
      //Inside component.js.tmpl
      import {
      BodyComponent
      } from 'mjml-core'
      extends BodyComponent {

    //Inside index.js var Chart = (0, _mjmlCore.BodyComponent)(_class = function (_Component) {

  6. Run npm test again. Still get the same error. This time it's coming from the file in src/index.js, which you say not to edit directly because it's generated with npm run update. From this comment, I would assume I can run npm run update but there is no script in the package.json. However, there is a script called "generate" which looks like will generate a new file of the component which may fix the test.

  7. Run npm run generate then run npm test. This is looking better. There is a DeprecationWarning from Mocha. Following the link, we learn that we can add --no-deprecation too remove this warning. So add that to the following scripts:

    • "test"
    • "test-watch"
  8. Run npm test again. 0 tests past. We see this error: TypeError: (0 , _mjmlCore.registerMJElement) is not a function - looks like this was changed in v4. Based on the current version, looks like this method changed to registerComponent so chagne that inside test/mjml-chart.spec.js

  9. Run npm test. Now we see 0/5 tests passing. The common error is _mjmlCore.MJMLRenderer is not a constructor. Looking at the current version, I believe this has been moved into a separate package called [mjml-validator`](https://www.npmjs.com/package/mjml-validator) so I install that as a dev-dependency.

  10. Run npm test again. Now the error we get is:

    TypeError: Cannot read property 'undefined' of undefined
      at validateAttribute (node_modules/mjml-validator/lib/rules/validAttributes.js:38:29)

    My guess is that it is trying to validate the attributes for mjml-chart but doesn't know where to look for the list of valid attributes? I'm a little stuck here. Not sure where to debug or how to solve this.

FGRibreau commented 6 years ago

Could you send a PR with your change? I will take a look

jsjoeio commented 6 years ago

Yeah, I'll do that now! And please let me know if I can help out more.

FGRibreau commented 6 years ago

fixed in #3