-
Notifications
You must be signed in to change notification settings - Fork 62
Enable resolving generated sandboxes.ts file for rootpath #70
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
Conversation
|
Could you provide more context for this PR? We look for the It seems more intuitive for |
|
So here's the deal. Take a look at this line of code here: |
|
Thanks for describing the problem in more detail. Can you give an example of such a non-standard directory structure? We use Playground internally with a multiple-app structure, e.g. As long as the |
|
Here's a demo of the bug for you to play with:
https://github.com/mrmeku/demo-playground-bug
Just clone the repo, do npm install and then try to npm run playground
…On Thu, Dec 7, 2017 at 5:32 PM Graham Marlow ***@***.***> wrote:
Thanks for describing the problem in more detail. Can you give an example
of such a non-standard directory structure? We use Playground internally
with a multiple-app structure, e.g. src/app1/app, src/app2/app. Each of
these "sub-apps" is outfitted with its own Playground that is configured
within a single .angular-cli.json.
As long as the .angular-cli.json root property is consistent between your
application and the playground application, I don't see why sandboxes.ts
wouldn't resolve properly. For instance, if your non-standard directory
root is somepath/other, make sure that both entries in .angular-cli.json
have "root": "somepath/other".
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHteOyce-BWLNSrd5zec1cHEhGh61R3Sks5s-IO5gaJpZM4Q6JjJ>
.
|
|
Thanks for putting that together. I'm going to spend some time looking into this issue--the latest CLI update has caused some problems with multi-app architectures. I've created a ticket on their repo that addresses one such problem, although the error you sent is new to me. |
|
Thanks for filing that. Lets see where it goes! If they aren't quick to fix
it you can always merge this PR as a temporary fix
…On Fri, Dec 8, 2017 at 11:38 AM Graham Marlow ***@***.***> wrote:
Thanks for putting that together. I'm going to spend some time looking
into this issue--the latest CLI update has caused some problems with
multi-app architectures; I've created a ticket
<angular/angular-cli#8809> on their repo that
addresses one such problem, although the error you sent is new to me.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHteO5HcgdUffsezt4Od6RV2Sw0w9Qxrks5s-YIzgaJpZM4Q6JjJ>
.
|
|
So I've done a lot of investigation into this issue and I thought I'd provide some findings. Angular CLI 1.5 update introduces a more strict adherence to the |
|
That makes sense to me, it's more or less what I thought the issue was. The
cli is in charge of the webpack config and now the config is more
restrictive than it was previously which broke angular playground. The fix
within my pull request does play well with the more restrictive webpack
config though. With that in mind do you want to merge it? Or do you have
another strategy for fixing this bug?
…On Thu, Dec 14, 2017, 10:58 AM Graham Marlow ***@***.***> wrote:
So I've done a lot of investigation into this issue and I thought I'd
provide some findings. Angular CLI 1.5 update introduces a more strict
adherence to the tsconfig.json, according to their release post
<https://blog.angular.io/version-5-0-0-of-angular-now-available-37e414935ced>.
Playground uses a project's existing CLI configuration to compile sandbox
chunks from the sandboxes.ts file. Therefore, it stands that the
increased strictness of tsconfig.json settings along with the issue that
I referenced above have led to some issues when using Playground with
non-default structures.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHteO4CB5-Cw6uOsLWwJTeCP_kX8-_xcks5tAWG4gaJpZM4Q6JjJ>
.
|
|
I am investigating other solutions to this problem that don't involve moving the |
|
One alternative I thought of would be to import it by it's absolute path.
You would just need to inject the app root flag into the typescript file
which imports the sandboxes. It would be a pretty simple change to make
other than exposing the flag
…On Thu, Dec 14, 2017, 1:32 PM Graham Marlow ***@***.***> wrote:
I am investigating other solutions to this problem that don't involve
moving the sandboxes.ts path. I tested it on some other application
structures and it didn't resolve the error in all cases.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHteO9qx35eLUwAEpGiZP4OBZLRsLCJHks5tAYXmgaJpZM4Q6JjJ>
.
|
|
Yeah that is certainly a consideration. That would require modifying the node_modules output at runtime though, since the sandboxes are resolved by webpack before it hits the angular application. |
|
Yeah, youd have to write a js file to import the path from. Certainly
doable, but certainly not as clean as you would hope for ideally
…On Thu, Dec 14, 2017, 6:11 PM Graham Marlow ***@***.***> wrote:
Yeah that is certainly a consideration. That would require modifying the
node_modules output at runtime though, since the sandboxes are resolved by
webpack before it hits the angular application.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHteO0fHnFuOGbaclSSUSTMv_O6jrkJ0ks5tAccrgaJpZM4Q6JjJ>
.
|
|
Awesome work! I love the solution you chose
…On Wed, Dec 27, 2017, 12:56 PM Graham Marlow ***@***.***> wrote:
@mrmeku <https://github.com/mrmeku> I've got a PR out that should resolve
this issue, #75 <#75>.
I decided not to do use this approach since the latest CLI version (1.6.2)
introduced some changes to the webpack configuration that altered the way
Playground was chunking out each sandbox individually. #75
<#75> fixes both
issues.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHteO3ll3gYqCATNSk98kmMfFoL2yylOks5tEqEHgaJpZM4Q6JjJ>
.
|
Before if you specified a custom root path angular-playground would be unable to find sandboxes.ts since it required the file to be generated at the root of a workspace. Sandboxes.ts is located using a require statement
require(sandboxes)and the esnext module resolution algorithm does not look within the specified root path for the module. To circumvent this, we place sandboxes.ts at the root of the workspace.