Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow package directories to be optional #80

Closed
andy-berry-dev opened this issue Oct 8, 2013 · 15 comments
Closed

allow package directories to be optional #80

andy-berry-dev opened this issue Oct 8, 2013 · 15 comments
Assignees
Milestone

Comments

@andy-berry-dev
Copy link
Member

instead of src/a/pkg/Class1.js I should be allowed to have src/Class1.js - the package namespace should still be inforced and calculated based on the app/bladeset/blade namespace.

@thecapdan
Copy link
Contributor

see #19 for AC

@thecapdan
Copy link
Contributor

Acceptance criteria:

1: Support simplified directory structure:

+- myblade
|   \- resources
|       \- html
|       \- 18n
|       \- xml
|       -- aliases.xml
|   \- src
|       -- MyBlade.js
|       -- MyClass.js
|   \- tests
|   \- themes
|   \- workbench

2: Ensure old structure is still supported.
3: Templates are updated to the new structure

Pull request: #893

@leggetter
Copy link
Contributor

👍

@thecapdan
Copy link
Contributor

Moving back to dev while the templates are updated. We currently still get packages when we create a new blade:

image

And bladeset:
image

Also in src at app level:
image

New templates should conform to the way shown in the comment above

@andy-berry-dev
Copy link
Member Author

The templates have now changed so package directories aren't used by default.

@thecapdan
Copy link
Contributor

Blade template now conforms to the new structure as does the bladeset - i.e. no package directories by default.

the app/src directory now has two files:

image

@andyberry88 was this necessary/intentional?

Also, while using this with ct libraries, we found that libraries that did not wish to have namespaced-enforcement had problems with bundling files under src-test folder while running tests. Andrew is aware of the issue - I will retest once fixed

@andy-berry-dev
Copy link
Member Author

@andyberry88 was this necessary/intentional?

Neither 😄 Its a side effect of having the default bladeset represent the same directory as the default aspect. I'll fix it.

@andy-berry-dev
Copy link
Member Author

@thecapdan Issues fixed in 50f515a and 9284471.

@thecapdan
Copy link
Contributor

Awesome. I'll take a look at these over the weekend. Thank you, sir, have a good one 👍

@thecapdan
Copy link
Contributor

Rogue js file is now gone. Default src directory for a freshly created app now looks like this:

image

following checkin 9284471

we have a test failing in CI: http://newgo.caplin.com/go/tab/build/detail/BRJS-brjs-develop/690/build/2/exampleAppTests

Seems to be ok when i run the same gradle command locally. Will need further investigation on Tuesday.

@thecapdan
Copy link
Contributor

@thecapdan Issues fixed in 50f515a and 9284471.

confirmed. both remaining issues have been fixed.

thecapdan pushed a commit that referenced this issue Aug 26, 2014
@leggetter
Copy link
Contributor

Given a "default" blade with the following directory structure:

blades/login
├── resources
├── src
│   └── modularapp
│       └── chat
│           └── login
│               └── LoginViewModel.js
├── tests
├── themes
└── workbench

When I view the default aspect I get the following error:

HTTP ERROR 500

Problem accessing /modularapp/en/. Reason:

    Server Error
Caused by:

java.lang.RuntimeException: org.bladerunnerjs.model.exception.InvalidRequirePathException: The source module at 'blades/login/src/modularapp/chat/login/LoginViewModel.js' is in an invalid location. It's require path starts with the apps require prefix ('modularapp') which suggests it's require path is intended to be 'modularapp/login'. The require path defined by the source modules location is 'modularapp/chat/login'. Either it's package structure should be 'modularapp/login/*' or remove the folders 'modularapp/chat/login' to allow the require prefix to be calculated automatically.
    ...

If I update the directory structure as follows:

blades/login
├── resources
├── src
│   ├── LoginViewModel.js
├── tests
├── themes
└── workbench

The error goes away.

Should the old style package structure be supported within default bladesets?

@andy-berry-dev
Copy link
Member Author

The require prefix for Blades inside the 'default' Bladeset is <app-require-prefix>/<blade-name>, this is different from Blades inside a named Bladeset where the require prefix is <app-require-prefix>/<bladeset-name>/<blade-name>.

So the exception is valid, the require prefix for your Blade should be modularapp/login.

@thecapdan
Copy link
Contributor

@leggetter can you confirm the change described above removes the error

@leggetter
Copy link
Contributor

@thecapdan Yep - this was my bad. I left a chat directory in there which was a hang over from the bladeset I migrated the blade from.

I've verified that with the correct legacy path things do work.

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

No branches or pull requests

3 participants