-
Notifications
You must be signed in to change notification settings - Fork 124
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
issue w/ plugins detective #495
Conversation
@matteofigus ... 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a little change. Everything else looks very good! Thanks mate!
test/unit/code.js
Outdated
@@ -0,0 +1 @@ | |||
module.exports = "module.exports=function(a){function b(d){if(c[d])return c[d].exports;var e=c[d]={i:d,l:!1,exports:{}};return a[d].call(e.exports,e,e.exports,b),e.l=!0,e.exports}var c={};return b.m=a,b.c=c,b.i=function(d){return d},b.d=function(d,e,f){b.o(d,e)||Object.defineProperty(d,e,{configurable:!1,enumerable:!0,get:f})},b.n=function(d){var e=d&&d.__esModule?function(){return d['default']}:function(){return d};return b.d(e,'a',e),e},b.o=function(d,e){return Object.prototype.hasOwnProperty.call(d,e)},b.p='',b(b.s=0)}([function(a,b){'use strict';Object.defineProperty(b,'__esModule',{value:!0});b.data=function(f,g){var h={logo:{href:'',url:f.params.logoUrl,alt:f.params.title+' logo'},title:f.params.title,timezoneOffset:'?'};f.plugins.getTimezoneOffset&&(h.timezoneOffset=f.plugins.getTimezoneOffset()),g(null,h)}}]);"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this file to /test/fixtures
? I think that's supposed to be the place for this kind of files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, although I like to keep related things together 😊 shall I move it to which subfolder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my preference would be to make it a component and bring it to /test/fixtures/mocked-components/missing-plugin.js
(look at the others in that dir to follow same convention)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matteofigus I will try to do that although what I do actually need here is only the generated code, but I see your point
@@ -1,6 +1,7 @@ | |||
'use strict'; | |||
|
|||
const expect = require('chai').expect; | |||
const code = require('../fixtures/mocked-components/package-server-with-plugin'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matteofigus what do you think about this? I only added the data
part and not the full object; you know that I'm a minimalist 😅
Description
as per #445 this functionality is broken; this PR tries to fix the
parse
logic. This, unfortunately, will not fix the fact that the Registry still will not throw any exception for the missing plugin(s); have a look at the issue mentioned above for more details.