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

Initial Implementation #2

Merged
merged 1 commit into from Aug 16, 2016

Conversation

Projects
None yet
4 participants
@MeirionHughes
Contributor

MeirionHughes commented Jun 21, 2016

Managed to rebase as a single commit; should be good to go.

@MeirionHughes

This comment has been minimized.

Show comment
Hide comment
@MeirionHughes

MeirionHughes Jun 22, 2016

Contributor

Working on RC, Tests passing.

Contributor

MeirionHughes commented Jun 22, 2016

Working on RC, Tests passing.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jun 29, 2016

Member

@MeirionHughes How do you feel about the status? Do you feel it's ready to merge? We'll need to fix it up to have a build process in-line with our other repos, but that aside, what do you think?

Member

EisenbergEffect commented Jun 29, 2016

@MeirionHughes How do you feel about the status? Do you feel it's ready to merge? We'll need to fix it up to have a build process in-line with our other repos, but that aside, what do you think?

@MeirionHughes

This comment has been minimized.

Show comment
Hide comment
@MeirionHughes

MeirionHughes Jun 29, 2016

Contributor

There are a few duplicated interfaces that, ideally, should be imported from aurelia-pal, But, given it needs cleaning to your own standards anyway... it might be better to run with it and change as you see fit. Up to you; its already published to npm (aureliaeffect has ownership) and it works fine for the linter. That said, I'm not using ViewCompiler; just the expression/binding reflection + DI container.

Contributor

MeirionHughes commented Jun 29, 2016

There are a few duplicated interfaces that, ideally, should be imported from aurelia-pal, But, given it needs cleaning to your own standards anyway... it might be better to run with it and change as you see fit. Up to you; its already published to npm (aureliaeffect has ownership) and it works fine for the linter. That said, I'm not using ViewCompiler; just the expression/binding reflection + DI container.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jul 11, 2016

Member

@martingust This represents the first steps toward server-side rendering. Can you do a review of the implementation?

@niieani Once we merge this, would you be able to help in fixing up the build process to match our other repositories? Since this code is fully written in TypeScript, we can use it as an opportunity to explore what our ideal TS-based build might look like.

Member

EisenbergEffect commented Jul 11, 2016

@martingust This represents the first steps toward server-side rendering. Can you do a review of the implementation?

@niieani Once we merge this, would you be able to help in fixing up the build process to match our other repositories? Since this code is fully written in TypeScript, we can use it as an opportunity to explore what our ideal TS-based build might look like.

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Jul 11, 2016

Member

@EisenbergEffect Sure! In fact, the last remaining use-case we had for Babel, i.e. transpilation to native-modules while still targetting es5, is supported as of TypeScript 2.0, so modifying our build implementation to fit should now be a breeze.

Also, I'd suggest to try and extract the build system out to a separate package, so that updating it in the future won't require us to change files in all the packages.

Member

niieani commented Jul 11, 2016

@EisenbergEffect Sure! In fact, the last remaining use-case we had for Babel, i.e. transpilation to native-modules while still targetting es5, is supported as of TypeScript 2.0, so modifying our build implementation to fit should now be a breeze.

Also, I'd suggest to try and extract the build system out to a separate package, so that updating it in the future won't require us to change files in all the packages.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jul 22, 2016

Member

@MeirionHughes Can you rebase this PR on the master branch? Thanks!

Member

EisenbergEffect commented Jul 22, 2016

@MeirionHughes Can you rebase this PR on the master branch? Thanks!

@MeirionHughes

This comment has been minimized.

Show comment
Hide comment
@MeirionHughes

MeirionHughes Jul 28, 2016

Contributor

Sorry for the delay; been a bit swamped with work and life. :P

Contributor

MeirionHughes commented Jul 28, 2016

Sorry for the delay; been a bit swamped with work and life. :P

@MeirionHughes

This comment has been minimized.

Show comment
Hide comment
@MeirionHughes

MeirionHughes Jul 28, 2016

Contributor

Its crazy how much can change in a few weeks... some of the stuff I'm doing in this commit I probably wouldn't do the same now. e.g. compiling the spec ts->js and then deleting the js afterward isn't needed, you can simply tell your IDE (VSCode) to ignore any js/map file when there is a corresponding ts file.

Contributor

MeirionHughes commented Jul 28, 2016

Its crazy how much can change in a few weeks... some of the stuff I'm doing in this commit I probably wouldn't do the same now. e.g. compiling the spec ts->js and then deleting the js afterward isn't needed, you can simply tell your IDE (VSCode) to ignore any js/map file when there is a corresponding ts file.

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Aug 12, 2016

Member

Hey @MeirionHughes, how is it going with the PR? I'd love to give this a try.
Big thanks for working on this.

Member

niieani commented Aug 12, 2016

Hey @MeirionHughes, how is it going with the PR? I'd love to give this a try.
Big thanks for working on this.

@MeirionHughes

This comment has been minimized.

Show comment
Hide comment
@MeirionHughes

MeirionHughes Aug 12, 2016

Contributor

Its been in limbo for a while. I think its waiting on someone from aurelia to take it and add it to your build.

I've published to npm already and https://www.npmjs.com/package/aurelia-template-lint is using it happily, so you could run with that if need be.

I suspect there is a targeting issue though because I've had people complain they cannot use node 4 with the linter.

Contributor

MeirionHughes commented Aug 12, 2016

Its been in limbo for a while. I think its waiting on someone from aurelia to take it and add it to your build.

I've published to npm already and https://www.npmjs.com/package/aurelia-template-lint is using it happily, so you could run with that if need be.

I suspect there is a targeting issue though because I've had people complain they cannot use node 4 with the linter.

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Aug 12, 2016

Member

@MeirionHughes can you explain what would be the next step with this now? What do you mean by "add it to your build"? I might be able to try it out when I have a bit more time.
@EisenbergEffect is there anyone in charge of Aurelia server-side generation?

Member

niieani commented Aug 12, 2016

@MeirionHughes can you explain what would be the next step with this now? What do you mean by "add it to your build"? I might be able to try it out when I have a bit more time.
@EisenbergEffect is there anyone in charge of Aurelia server-side generation?

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Aug 12, 2016

Member

Martin is, sort of. But no one has really taken the lead just yet. If you want to handle this, that would be great. I think the PR just needs to be modified to have a property build setup and repo structure.

Member

EisenbergEffect commented Aug 12, 2016

Martin is, sort of. But no one has really taken the lead just yet. If you want to handle this, that would be great. I think the PR just needs to be modified to have a property build setup and repo structure.

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Aug 12, 2016

Member

To make server-side generation optimal I think it would be a good idea to also make an aurelia-loader-nodejs (or better: enhance the default loader) to support NodeJS sync-style require() dependency loading. It'll be useful for Electron too. I can look into that, possibly also server-side generation, but I've got a lot on my plate already anyway.

Member

niieani commented Aug 12, 2016

To make server-side generation optimal I think it would be a good idea to also make an aurelia-loader-nodejs (or better: enhance the default loader) to support NodeJS sync-style require() dependency loading. It'll be useful for Electron too. I can look into that, possibly also server-side generation, but I've got a lot on my plate already anyway.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Aug 12, 2016

Member

Feel free to look into that but don't feel obligated. Martin will be jumping in on this soon as well.

Member

EisenbergEffect commented Aug 12, 2016

Feel free to look into that but don't feel obligated. Martin will be jumping in on this soon as well.

@martingust

This comment has been minimized.

Show comment
Hide comment
@martingust

martingust Aug 16, 2016

Member

@EisenbergEffect @niieani @MeirionHughes I have read through the code and it looks pretty good to me and we can iterate over it along the way server side generation gets implemented. If we merge this I can modify the repo structure. @EisenbergEffect what do you mean by "a property build"? @niieani Assuming you still have a lot on your plate (?) I can start looking at the loader and server side generation and collaborate the implementation with you?

Member

martingust commented Aug 16, 2016

@EisenbergEffect @niieani @MeirionHughes I have read through the code and it looks pretty good to me and we can iterate over it along the way server side generation gets implemented. If we merge this I can modify the repo structure. @EisenbergEffect what do you mean by "a property build"? @niieani Assuming you still have a lot on your plate (?) I can start looking at the loader and server side generation and collaborate the implementation with you?

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Aug 16, 2016

Member

I think that was a typo and I meant "proper build". I'll go ahead and merge this so we can move forward with getting things in place and working on server render.

Member

EisenbergEffect commented Aug 16, 2016

I think that was a typo and I meant "proper build". I'll go ahead and merge this so we can move forward with getting things in place and working on server render.

@EisenbergEffect EisenbergEffect merged commit 0badcf7 into aurelia:master Aug 16, 2016

@niieani

This comment has been minimized.

Show comment
Hide comment
@niieani

niieani Aug 16, 2016

Member

@martingust that sounds good. I'm still quite busy, but if you start something ping me and I'll see how much time I can invest into it.
I have some great ideas for server-rendering use-cases, e.g. building and generating static sites using Aurelia's components, something that Gatsby is trying to do for React. If done right, I think this could really popularize Aurelia.

Member

niieani commented Aug 16, 2016

@martingust that sounds good. I'm still quite busy, but if you start something ping me and I'll see how much time I can invest into it.
I have some great ideas for server-rendering use-cases, e.g. building and generating static sites using Aurelia's components, something that Gatsby is trying to do for React. If done right, I think this could really popularize Aurelia.

@MeirionHughes MeirionHughes deleted the MeirionHughes:rebase branch Aug 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment