feat: load modelina configuration options from a config file #1367

kaushik-rishi · 2023-06-03T04:35:32Z

Description

WIP PR
Related issue(s)
Modelina config file #780

netlify · 2023-06-03T04:35:40Z

✅ Deploy Preview for modelina canceled.

Name	Link
🔨 Latest commit	`a207ca2`
🔍 Latest deploy log	https://app.netlify.com/sites/modelina/deploys/654d399f1eba530008219ae1

coveralls · 2023-06-03T04:40:45Z

Pull Request Test Coverage Report for Build 5161878796

0 of 5 (0.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.08%) to 92.12%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/helpers/FileHelpers.ts	0	5	0.0%

Totals
Change from base Build 5156178606:	-0.08%
Covered Lines:	5247
Relevant Lines:	5554

💛 - Coveralls

kaushik-rishi · 2023-06-03T04:44:08Z

@jonaslagoni I've used the require function to import the object exported by the config file.
Post build I've observed that the loadConfigFile function is only present in /lib/cjs and not lib/esm.

Why is it that way ? 🤔

Or, do you want me to import the module like this ?

function readJSFile(filePath) {
  try {
    const fileContent = fs.readFileSync(filePath, 'utf8');
    const moduleExports = {};
    const moduleWrapper = eval('(function(exports) {' + fileContent + '})');
    moduleWrapper(moduleExports);
    return moduleExports;
  } catch (error) {
    console.error('Error reading JS file:', error);
    return null;
  }
}

jonaslagoni · 2023-06-04T22:03:17Z

Post build I've observed that the loadConfigFile function is only present in /lib/cjs and not lib/esm.

Which build command did you run?

modelina/package.json

Line 78 in c0e0268

"build:esm": "tsc --project tsconfig.json --module ESNext --outDir ./lib/esm",

kaushik-rishi · 2023-06-05T04:08:03Z

Which build command did you run?

On running build:esm, i do find the function loadConfigFile.
But previously i think i ran build command, i didn't find the function loadConfigFile in the directory /lib/esm, irrespective it's working now. 🙂

Thanks. 😄

Do you think we should add more documentation on setting up the repository for development ?

kaushik-rishi · 2023-06-05T10:08:27Z

@jonaslagoni After i add a function, and build the project (build:esm).
How do i test the imports and stuff ?
Like let's say i create a new project somewhere else in my Desktop, how do i import the newly build version of this project 🤔 ?

Edit:
[RESOLVED]

kaushik-rishi · 2023-06-06T06:30:48Z

@jonaslagoni I'm kind of stuck / plateaued here. 🙂
What should i further do ? 🤔
Is there any other validation we want to run on the imported config file ?

I'm talking feature wise, i haven't written the tests yet.

sonarqubecloud · 2023-06-06T06:38:06Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
46.1% Duplication

jonaslagoni

Generally, you need to add documentation as well, probably something like the following:

Add a section here: https://github.com/asyncapi/modelina/blob/master/docs/advanced.md
Add your example to the list of examples: https://github.com/asyncapi/modelina/blob/master/examples/README.md

Hope this gives you a direction, otherwise let me know if you want anything clarified 🙂

jonaslagoni · 2023-06-06T14:06:02Z

src/helpers/FileHelpers.ts

@@ -64,4 +64,18 @@ export class FileHelpers {
      }
    });
  }
+
+  static async getConfigFromFile(configPath: string): Promise<any> {


You have to write some unit tests here 🙂

Make sure you test error states as well:

What if the file exist but is not valid JS?

What if the file exist but is TypeScript?

What if the file exist and is correct JS, but does not expose the right config?

What if the file exist and is correct JS, but does not expose the config file the right way?

Make sure you document this function as well i.e. what it does 🙂

@jonaslagoni
I'll make sure to document and write the unit tests.

What if the file exist and is correct JS, but does not expose the right config?

and

What if the file exist and is correct JS, but does not expose the config file the right way?

Should be the same thing right ? I mean we wouldn't know if the file is exposing the config the right way, unless we know what it's exposing ? 🤔

The way i put it here might be confusing, feel free to ignore this, i will let you know later more context on this. 🙂

I'll try fixing things one after other and contact you back 😄

The way i put it here might be confusing, feel free to ignore this, i will let you know later more context on this. 🙂

The first one is a bit hard to pull off, which is they expose the configuration correctly, but the configuration is incorrect (not sure we actually can do this)

The second one is if they expose it like export { config2: config }; when we expected export config;.

The first one is a bit hard to pull off, which is they expose the configuration correctly, but the configuration is incorrect (not sure we actually can do this)

I'll try to see if we can do the first one

The second one is if they expose it like export { config2: config }; when we expected export config;.

I'll look into this as well 🙂
Seems like a good learning oppurtunity.

@jonaslagoni

We can restrict the users to only allow them to use js file for config right ? i hardly saw places where people used ts files for config.
What do you say ?

Agree 👍

It can also be an improvement down the line if ever requested 🙂

jonaslagoni · 2023-06-06T14:07:15Z

examples/get-config-from-file/index.ts

+    const config: { config: TypeScriptOptions } =
+      await FileHelpers.getConfigFromFile('./modelina.config.alternate.js');


I think we should simplify this.

const config = await FileHelpers.getConfigFromFile('./modelina.config.alternate.js');

Instead of having to do something like config.config, what do you think? 🤔

Yeah definitely, it looks absurd.
We can directly export the config. 🙂
Would that work ?

kaushik-rishi · 2023-06-06T14:53:53Z

@jonaslagoni

The config file can be placed anywhere on the users machine, which is probably not a node package and doensn't have modelina installed.

Out getConfigFromFile function will be called let's say from cli, where modelina will be installed, so the require should work right ?

kaushik-rishi · 2023-06-12T10:16:29Z

I will pause working on this PR for 2-3 days, busy with some other things

jonaslagoni · 2023-08-16T11:31:57Z

Looks like you are nearly there @kaushik-rishi 🤔

sonarqubecloud · 2023-11-09T19:58:02Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
4.4% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

jonaslagoni · 2024-01-02T18:30:36Z

Hey @kaushik-rishi, want to continue or can someone else take over the progress?

jonaslagoni · 2024-02-19T10:02:04Z

Closing as no response for over a month

wip: file helper for loading config

e7d4377

kaushik-rishi requested review from magicmatatjahu, jonaslagoni, kennethaasan and asyncapi-bot-eve as code owners June 3, 2023 04:35

kaushik-rishi changed the title ~~wip: file helper for loading config~~ feat: load modelina configuration options from a config file Jun 3, 2023

kaushik-rishi and others added 4 commits June 5, 2023 20:14

add example for get config from file

89a24c0

Merge branch 'master' into feat-modelina-config

69c83ab

add error handling for getConfigFromFile

a16426a

add another example config file

e53bc72

kaushik-rishi marked this pull request as draft June 5, 2023 17:53

kaushik-rishi and others added 2 commits June 6, 2023 12:06

add documentation for get-config-from-file example

fc9d9fe

Merge branch 'master' into feat-modelina-config

e95d6c6

jonaslagoni requested changes Jun 6, 2023

View reviewed changes

jonaslagoni added 2 commits October 26, 2023 12:53

Merge branch 'master' into feat-modelina-config

3c5331e

Merge branch 'master' into feat-modelina-config

a207ca2

jonaslagoni closed this Feb 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: load modelina configuration options from a config file #1367

feat: load modelina configuration options from a config file #1367

kaushik-rishi commented Jun 3, 2023

netlify bot commented Jun 3, 2023 •

edited

Loading

coveralls commented Jun 3, 2023

kaushik-rishi commented Jun 3, 2023

jonaslagoni commented Jun 4, 2023

kaushik-rishi commented Jun 5, 2023

kaushik-rishi commented Jun 5, 2023 •

edited

Loading

kaushik-rishi commented Jun 6, 2023 •

edited

Loading

sonarqubecloud bot commented Jun 6, 2023

jonaslagoni left a comment

jonaslagoni Jun 6, 2023

jonaslagoni Jun 6, 2023

kaushik-rishi Jun 6, 2023

kaushik-rishi Jun 6, 2023

jonaslagoni Jun 6, 2023

kaushik-rishi Jun 6, 2023

kaushik-rishi Jun 6, 2023

jonaslagoni Jul 28, 2023

jonaslagoni Jun 6, 2023

kaushik-rishi Jun 6, 2023

jonaslagoni Jun 6, 2023

kaushik-rishi commented Jun 6, 2023

kaushik-rishi commented Jun 12, 2023

jonaslagoni commented Aug 16, 2023

sonarqubecloud bot commented Nov 9, 2023

jonaslagoni commented Jan 2, 2024

jonaslagoni commented Feb 19, 2024

		const config: { config: TypeScriptOptions } =
		await FileHelpers.getConfigFromFile('./modelina.config.alternate.js');

feat: load modelina configuration options from a config file #1367

feat: load modelina configuration options from a config file #1367

Conversation

kaushik-rishi commented Jun 3, 2023

netlify bot commented Jun 3, 2023 • edited Loading

✅ Deploy Preview for modelina canceled.

coveralls commented Jun 3, 2023

Pull Request Test Coverage Report for Build 5161878796

💛 - Coveralls

kaushik-rishi commented Jun 3, 2023

jonaslagoni commented Jun 4, 2023

kaushik-rishi commented Jun 5, 2023

kaushik-rishi commented Jun 5, 2023 • edited Loading

kaushik-rishi commented Jun 6, 2023 • edited Loading

sonarqubecloud bot commented Jun 6, 2023

jonaslagoni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaushik-rishi commented Jun 6, 2023

kaushik-rishi commented Jun 12, 2023

jonaslagoni commented Aug 16, 2023

sonarqubecloud bot commented Nov 9, 2023

jonaslagoni commented Jan 2, 2024

jonaslagoni commented Feb 19, 2024

netlify bot commented Jun 3, 2023 •

edited

Loading

kaushik-rishi commented Jun 5, 2023 •

edited

Loading

kaushik-rishi commented Jun 6, 2023 •

edited

Loading