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

Enables multiple entry points nested in rails root #875

Closed
wants to merge 1 commit into from
Closed

Enables multiple entry points nested in rails root #875

wants to merge 1 commit into from

Conversation

lazylester
Copy link
Contributor

With this change, webpacker config file can contain (e.g.) source_path: "**/app/javascript". This creates packs for all entry points below the rails roots, and specifically for the engines in vendor/gems.

If this change is not present, then the keys of manifest.json are malformed due to the globbed source path.

I would be happy to submit a test for this, if you can suggest how to test it. It doesn't break the existing tests, but the affected code may not be covered by the test suite.

With this change, webpacker config file can contain (e.g.) source_path: "**/app/javascript". This creates packs for all entry points below the rails roots, and specifically for the engines in vendor/gems.

If this change is not present, then the keys of manifest.json are malformed due to the globbed source path.

I would be happy to submit a test for this, if you can suggest how to test it. It doesn't break the existing tests, but the affected code may not be covered by the test suite.
@mltsy
Copy link

mltsy commented Sep 28, 2017

Hmm... I'm confused as to why prepending the rails root directory would result in an invalid manifest.json... isn't /rails/root/**/app/javascripts a valid glob path that resolves into the same number of valid keys as **/app/javascripts? Is there a way you could easily demonstrate this failure with a unit test? (does this method output something that would be invalid in manifest.json when using globs? Or is the failure caused by the output here interacting with something else?)

@gauravtiwari
Copy link
Member

@lazylester Thanks for this, but I am unsure what you are trying to achieve here. The code you removed is there to enable nested namespaces within packs so probably this needs bit more thinking.

@lazylester
Copy link
Contributor Author

@gauravtiwari I inferred namespaces from the name of the variables. Seems like a good idea. Since my change didn't break any tests, I could not look in the test suite to see what the intended behaviour should be.

What I'm trying to achieve is for packs to be compiled from engines within vendor/gems, and the compiled packs to be available in public/packs and correctly referenced by keys in manifest.json.

I imagined that declaring the source_path in config/webpacker.yml as "/app/javascripts" would achieve my objective, and it nearly does. It compiles all the packs that the "/app/javascripts" finds (main app, and engines within vendor/gems). However the keys of the manifest produced by the compilation do not resolve to paths within root/public. Without my proposed change, the manifest.json keys point to file paths within the engines, whereas they should point to file paths within public, if my understanding is correct.

@lazylester
Copy link
Contributor Author

@gauravtiwari sorry I didn't articulate very clearly in my last response.
without my proposed change manifest.json has keys that look like (for example, in my app that has an engine called Complaint) this:
"../../../../vendor/gems/complaints/app/javascript/packs/complaints.js"

but when I wish to insert this file with a javascript_pack_include tag, I should just call 'complaints', as I would if it were a javascript_include tag.

Hopefully that's clearer!

@mltsy
Copy link

mltsy commented Sep 29, 2017

This is the issue we're trying to solve: #348 (comment)
(A way to make modules from an engine using webpacker automatically compile when the parent app's packs are compiled, and available to javascript_include as mentioned).

@lazylester's idea was to use **/app/javascripts in the source_path option of webpacker.yml, but that apparently produced an invalid manifest.json. Another option would be to allow multiple entries for the sources option - or perhaps automatically find/include packs from any engines using webpacker? Do you have any other suggestion for how to do this?

@gauravtiwari
Copy link
Member

@lazylester Thanks for explaining the use case however, I am afraid that this change isn't enough (due to varied use cases) and is also potentially breaking the name-spacing feature under packs so, I am going to close this. Hopefully will take a look at engine integration soon.

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