Skip to content

Conversation

abhi12706
Copy link
Contributor

No description provided.

const archive = archiver('zip', { zlib: { level: COMPRESSION_LEVEL } });

output.on('close', function () {
console.log(`Successfully zipped out folder to out.zip`);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully created [name].zip

import { readFileSync } from 'fs';
import { resolve } from 'path';

const errors = [];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not be a global variable.

const requiredFields = ['id', 'title', 'url', 'scopes'];
const optionalFields = ['props'];

checkRedundantFields(config, requiredFields, optionalFields);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass args as an object

const optionalFields = ['props'];

checkRedundantFields(config, requiredFields, optionalFields);
validateRequiredFields(config, requiredFields, true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass args as an object

function validateRequiredFields({ config, requiredFields, isWidget }) {
const errors = [];

requiredFields.forEach(field => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can use reduce?

const path = resolve(process.cwd(), 'manifest.json');
const config = JSON.parse(readFileSync(path));

try {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is try catch needed?

};

const validateProps = props => {
const errors = [`Value of field props should be an object`];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field props of ___ should be an object

Object.entries(config).reduce((acc, [key]) => {
if (!requiredFields.includes(key) && !optionalFields.includes(key)) {
acc.push(
`Field '${key}' should be removed from ${

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be removed from <> not supported

if (!requiredFields.includes(key) && !optionalFields.includes(key)) {
acc.push(
`Field '${key}' should be removed from ${
isWidget !== undefined ? `${isWidget ? 'widget' : 'page'} "${config.title}"` : 'manifest object'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too complicated. move to a function. return full strings

if (!(field in config)) {
acc.push(`Field '${field}' is required in manifest object`);
} else if (field === 'integrationType' && !INTEGRATION_TYPES.includes(config[field])) {
acc.push(`Field ${field} should be one of ${INTEGRATION_TYPES}`);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already know if its a react type widget of iframe. user doesnt need to choose. it should be according to the widget type

@abhi12706 abhi12706 changed the title feat: Validations and zip scripts added for react-component feat: Validations and zip scripts added Dec 8, 2023
scripts/zip.js Outdated
const path = resolve(process.cwd(), 'manifest.json');
const config = JSON.parse(readFileSync(path));

const zipFileName = config.name + ' ' + config.version + '.zip';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

template literal?


const appType = '<%=appType%>';

const INTEGRATION_TYPES = { 'React Component': 'VIRTUALIZED', iFrame: 'CONTAINERIZED' };

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we put all these keys, values in constants and use that everywhere?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we rename this to something like APP_TYPE_VS_INT_TYPE?

};

const checkRedundantFields = ({ config, requiredFields, optionalFields, isWidget }) =>
Object.entries(config).reduce((acc, [key]) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.keys() ?

return acc;
}, []);

const validateManifestOptionalFields = (config, optionalFields) =>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we reduce over config and remove optionalFields arg?

const validateManifestOptionalFields = (config, optionalFields) =>
optionalFields.reduce((acc, field) => {
if (field === 'basePath') {
if (field in config && !(typeof config[field] === 'string' && isUrlValid(config[field]))) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just need !isUrlValid check?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also we can use exhaustive switch statement here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants