LearnNewsExamplesServices
Frontmatter
titlefeat(build): allow create-class to drop classes into cwd
authorThorstenSuckow
stateMerged
createdAtJun 24, 2022, 3:19 PM
updatedAtJul 11, 2022, 6:39 PM
closedAtJun 25, 2022, 11:13 AM
mergedAtJun 25, 2022, 11:13 AM
branchesdevdev
urlhttps://github.com/neomjs/neo/pull/3193
Merged
ThorstenSuckow
ThorstenSuckow commented on Jun 24, 2022, 3:19 PM

create-class creates class files in current working directory

This PR introduces several new enhancements to the create-class script:

  1. new option for dropping classes directly into the cwd: -d
  2. option for specifying the app-source folder for computing namespace when -d is used: -s
  3. script available as binary via npx neo-cc (test w/ npm i -g for repository clones)
  4. several minor enhancements for original source related to path computing

Changes

The original implementation remains unchanged regarding functionality, some lines had to be shifted and re-arranged into conditional statements given the new option the script provides. It was also necessary to distinguish between - and further reuse - the original cwd- and __dirname-variables, so the script can now be run outside of the neo-folder.

Features

-d Option

The -d options stands for drop and advises the script to run in a none-interactive environment. It will compute the namespace based on the current working directory up to the directory that is either used from the value found in the script's sourceRootDirs-array, or specified with the -s-option.

Example:

The user has an application in /neo/apps/acme and wants to create a new class for acme.view.masterDetail.TableView.

cd /neo/apps/acme/view/masterDetail
npx neo-cc -c TableView -b component.Base -d

results in a file TableView.mjs created at /neo/apps/acme/view/masterDetail with the following contents:

import Base from '../../../../src/component/Base.mjs';

/**
 * @class acme.view.masterDetail.TableView
 * @extends Neo.component.Base
 */
class TableView extends Base {
    static getConfig() {return {
        /*
         * @member {String} className='acme.view.masterDetail.TableView'
         * @protected
         */
        className: 'acme.view.masterDetail.TableView',
        /*
         * @member {Object} _vdom
         */
        _vdom:
        {}
    }}
}

Neo.applyClassConfig(TableView);

export default TableView;

Note, that apps is by default mentioned in the sourcesRootDir of the script. The user can overwrite the directory where the sources (and therefore top-level namespaces reside) by using the -s option:

Example:

The user has an application in /neo/FOLDER/acme and wants to create a new class for acme.view.masterDetail.TableView.

cd /neo/FOLDER/acme/view/masterDetail
npx neo-cc -c TableView -b component.Base -d -s FOLDER

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)
tobiu
tobiu commented on Jun 25, 2022, 11:13 AM

Hi @ThorstenSuckow, of course i will accept the PR :)

Just 2 comments:

  1. I don't think we should add a license on a "per file" basis. We have the LICENSE file on the top level of the repo, which github and npm recognises. Adding it inside every file would increase the repo-size quite a bit and if we want to change the wording a little bit at some point, it would be quite some effort. We could create a new ticket to discuss about it though.
  2. App namespaces are most likely written in PascalCase. E.g. acme would probably be AcMe or something similar. Since the folder name of an app does not need to know about uppercase letters, there are 2 files: https://github.com/neomjs/neo/blob/dev/buildScripts/webpack/json/myApps.template.json and a version without the .template as soon as you create a new app. this file also exist inside the workspace scope. As a follow-up ticket, we should parse the file(s) and adjust the class names accordingly.

ThorstenSuckow
ThorstenSuckow commented on Jun 25, 2022, 5:30 PM
  1. I don't think we should add a license on a "per file" basis. We have the LICENSE file on the top level of the repo, which github and npm recognises. Adding it inside every file would increase the repo-size quite a bit and if we want to change the wording a little bit at some point, it would be quite some effort. We could create a new ticket to discuss about it though.

ACK.

  1. App namespaces are most likely written in PascalCase. E.g. acme would probably be AcMe or something similar. Since the folder name of an app does not need to know about uppercase letters, there are 2 files: https://github.com/neomjs/neo/blob/dev/buildScripts/webpack/json/myApps.template.json and a version without the .template as soon as you create a new app. this file also exist inside the workspace scope. As a follow-up ticket, we should parse the file(s) and adjust the class names accordingly.

I don't think I understand you correctly - how does this affect the proposed changes?

Also, the usage of PascalCase instead of camelCase in namespace parts worries me since I'm not used to it ;) , but I will create a discussion out of it.


Dinkh
Dinkh commented on Jul 11, 2022, 6:38 PM

I am in for PascalCase ;) The app folder is lowercase. The NameSpace is PascalCase.