Add module type support in i18next framework output file #119
Conversation
… JSON formatters - Updated `JSONFormatter` and `BaseFramework` to use `outDir` instead of `outputDir` for improved consistency. - Introduced module type handling in `I18NextFramework` to support both CommonJS and ES module formats. - Added methods for generating require statements and module exports to streamline output generation. - Enhanced Zod schema for JSON output to include optional module type specification.
There was a problem hiding this comment.
The naming convention (which we did not establish) is confusing, because one has type "module" while the other includes the actual word "module" in one of its strings:
- CommonJS = require, module.exports
- Module = import, export
When I saw that the codegenModuleExports method was not in the part of the condition for type "module", I thought you had written the condition backwards, but you hadn't. It's just a confusing naming convention.
We didn't establish the naming conventions for commonjs vs module which create the issue, but we do have the power to name and write comments for our own methods.
I think it would be helpful to rename some of the javascriptCodegenMixin methods to incorporate the "type" name and add a docstring comment for each with an example of the string it returns for help not mixing up the methods.
I can go ahead and push up those changes for ya.
| return `import ${module} from "${moduleName}";\n`; | ||
| } | ||
|
|
||
| protected codegenDefaultRequire(module: string, moduleName: string) { |
There was a problem hiding this comment.
Do we want to make a codeNamedRequire method as well? You mentioned that we're not currently using codegenNamedImport but if we're going to keep that method around, I would expect us to have the corresponding module method. If we don't think they will be needed, then I think we should delete the existing unused one.
There was a problem hiding this comment.
I went ahead and added it - if we don't think they'll ever be of use, we can delete them both instead.
|
Yeah go ahead and make the changes Marla! I totally agree they picked a confusing naming convention haha |
- Add docstring comments to codegen methods - Add type to some additional docstring comments - Rename one codegen method - Add named require method
marla-hoggard
left a comment
marla-hoggard
left a comment
There was a problem hiding this comment.
I pushed up the changes that I mentioned - take a look and if you like it, go ahead and merge!
jaerod95
deleted the
jrod/dit-10130-add-support-for-setting-output-js-file-into-commonjs-or
branch
Overview
Adds support for a
typefield used by the i18next framework to generate commonjs or ES6 module syntax in the generated javascript driver file. Defaults to commonjsContext
JSONFormatterandBaseFrameworkto useoutDirinstead ofoutputDirfor improved consistency.I18NextFrameworkto support both CommonJS and ES module formats.Screenshots
Test Plan
Testing successfully completed in via:
i18nextshould generate a javascript file with commonjs syntax.typeproperty with a value ofcommonjsin the json output with the framework set toi18nextshould generate a javascript file with commonjs syntax. (equivalent of the first bullet point.typeproperty with a value ofmodulein the json output with the framework set toi18nextshould generate a javascript file with module syntax.