Eslint: Critical dependency
craigcoles opened this issue · 8 comments
When using:
import * as RJX from "rjx/dist/rjx.esm";The following warning is displayed with ESlint:
Hi Craig is this still an issue?
Hi Yaw, I am still seeing this issue. If there is anything else that I can help with, let me know 👍
@yawetse - The problem's being caused by this line:
https://github.com/repetere/rjx/blob/38cff27da784d3156831be231eff770e80af955e/src/main.js#L115
Not sure what the __express function is for, but the require statement shouldn't accept a variable.
Great catch @petewalker.
The __express function is so express will recognize RJX as a view engine in an express app and render a .rjx file as a view. The filePath is the reference to the view Express renders.
I see - in which case, are you not better off using fs.readFile? Something like (not tested):
import fs from 'fs'
/**
* Use RJX for express view rendering
* @param {string} filePath - path to rjx express view
* @param {object} options - property used for express view {locals}
* @param {object} options.__boundConfig - property used to bind this object for RJX, can be used to add custom components
* @param {string} [options.__DOCTYPE="<!DOCTYPE html>"] - html doctype string
* @param {*} callback
*/
export function __express(filePath, options, callback) {
try {
const resources = Object.assign({}, options);
delete resources.__boundConfig;
delete resources.__DOCTYPE;
delete resources.__rjx;
const performRender = (rjxModule) => {
const rjxRenderedString = rjxHTMLString.call(options.__boundConfig || {}, {
rjx: rjxModule,
resources,
});
return callback(null, `${options.__DOCTYPE || '<!DOCTYPE html>'}
${rjxRenderedString}`);
};
if (options.__rjx) {
performRender(options.__rjx);
} else {
fs.readFile(filePath, (err, content) => {
if (err) return callback(err)
return performRender(content);
})
}
} catch (e) {
callback(e);
}
}@petewalker yes that's a good solution.
There are a couple of things to change if we were to go that route, the being loaded should be evaluated through a VM. Technically that .rjx file could be a regular JS file, or a JSON file (which is why originally I used require).
I'm happy to push out a slightly modified version of what you just included in your comment. but if you want to make a PR and add the VM parsing that would be helpful!
Thanks again for digging into this.
@yawetse I can take a look - I'm not familiar with express's view engines, but I'm sure I can knock together a simple test case.
However, I'm a little confused about what the contents of a .rjx file should be - you mentioned it could be JS or JSON. Under what circumstances would you want an RJX file to contain JavaScript? Surely the beauty of RJX is that it's JS free..?
Is there any documentation on what should be in an RJX file? (I couldn't find any...)
Hi @petewalker, unfortunately, no I haven't had a chance to document what should be in an RJX File.
But to clarify the intent, it was to be a drop in replacement for using EJS in express. So like EJS which is just javascript that renders a string as an express view. An RJX file is just a JavaScript file that exports JSON (the JSON used to render the React Components).
I think to keep the spec super straightforward if the extension is .rjx assume it's a JSON file (that contains the RJX for rendering the view). If the extension is anything other than .rjx assume it's a javascript file that exports a JavaScript Object that contains the RJX for rendering the view.
i.e.
some-express-view.rjx
{
"div":{
"props":{ "style":{ "color":"red", } },
"children":"hello red"
}
}some-express-view.js
const viewdata = [1,2].map(num => { p: { children:`number ${num}` } })
module.exports = {
div: {
children:viewdata
}
}