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

export modular-views.macro #59

Closed
threepointone opened this issue Aug 30, 2020 · 4 comments · Fixed by #140
Closed

export modular-views.macro #59

threepointone opened this issue Aug 30, 2020 · 4 comments · Fixed by #140
Assignees
Labels
enhancement New feature or request high priority

Comments

@threepointone
Copy link
Contributor

The current implementation exposes the internals of modular, and requires installing codegen.macro. This should probably be inside a module. Further, following #49, it'll probably have some 'proprietary' logic, and will be hard to change across releases if we don't encapsulate it.

Opening this issue as a stub, will decide an implementation after #49 lands.

@threepointone
Copy link
Contributor Author

Honestly, there's no reason this has to be included under modular, since it isn't really modular specific. I'm not even suggesting this should be a separate npm module, but that maybe this should just be a jpm specific thing.

To counter this notion, we should think of benefits that would only be unlocked by having widgets as a first class concept in a modular project. (As it stands, not much? And maybe that's not a bad thing?)

@threepointone threepointone changed the title feature: modular-scripts/widgets.macro package: modular-scripts/widgets.macro Aug 31, 2020
@threepointone threepointone mentioned this issue Sep 2, 2020
@sebinsua
Copy link
Contributor

sebinsua commented Sep 2, 2020

Agree with this.

It's gross code and proprietary. But I don't have a good solution right now: I don't want it to be manual commands, I didn't want to automate it with a Webpack plugin, and I don't like that I had to use a Babel macro.

To counter this notion, we should think of benefits that would only be unlocked by having widgets as a first class concept in a modular project. (As it stands, not much? And maybe that's not a bad thing?)

Another option would be literally writing the code which creates each lazy widget component manually.


Edit: Btw, an issue with putting a Babel macro into a package, is that it would not run 'by default' within node_modules/ unless CRA was setup to run these extra plugins against that directory. I think it only runs preset-env on node_modules/...

@threepointone threepointone added the enhancement New feature or request label Sep 6, 2020
@threepointone threepointone changed the title package: modular-scripts/widgets.macro export modular-scripts/widgets.macro Sep 6, 2020
@sebinsua
Copy link
Contributor

sebinsua commented Oct 5, 2020

This relates in some ways to the high-level 'loaders' issue.

@threepointone
Copy link
Contributor Author

1.0

threepointone added a commit that referenced this issue Nov 11, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 11, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 11, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 11, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details -

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125.
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were:
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project.
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case.
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR.

- I need to implement and write tests for the views macro, as mentioned above.
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so.
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait.

I'm looking for a review so I can start making changes in parallel to the above fixes. Have a look!
threepointone added a commit that referenced this issue Nov 12, 2020
* refactor

This PR is a refactor of the codebase, but without changing (much) functionality. It may seem like a lot, but that's exaggerated by the fact that a lot of it is just moved round. Here are the details - 

- This updates `react-scripts`/`@craco/craco` to support `create-react-app@4.0.0`. As part of that, I've also disabled the in-browser linting experience (dilanx/craco#205). *This is temporary*. I'll enable it when it's fixed on craco's side, but that shouldn't block this PR. This should fix #125. 
- The refactor is mainly around making this repository self-hosted (#120). The steps taken to achieve that were: 
  - e2e-tests were split up and inlined into `modular-scripts` / `create-modular-react-app`, as applicable. This also enabled removing verdaccio and associated complexity.
  - `cra-template-modular-typescript` was inlined into `modular-scripts`; this has the benfit of preventing one waterfall step when adding a new app to a project. 
  - We had a bug where we needed at least one app named `app` for all tests to run, I fixed that. We still need at least one app, but it can be named whatever. (I've named it `modular-site`, and we can use it for experiments, and maybe hosting documentation). In the future we dhsould fix it so it doesn't need any at all, but that's an edge case. 
- I've removed `modular-scripts/src/getModularViewMap`, and will instead implement `modular-views.macro` (#59). I'm working on it right now, but it shouldn't block this PR. 

### caveats 

- I need to implement and write tests for the views macro, as mentioned above. 
- While this runs all tests on CI, it skips one test, the one that starts up a server and tests whether the app works as expected. This is because it still leaves hanging processes, and will eat up our github-actions minutes. Working on fixing it, it's a weird one. (probably related to #54)
- I haven't verified builds yet, will do so. (EDIT: done, looks fine)
- I need to simplify the root configurations for babel/eslint so that it uses the `modular` configs. It's not critical to this PR so I figured it could wait. 
- all tests take about 550s on my 2015 macbook pro. That kinda sucks. A lot of the time is literally just fetching dependencies and installing. We should setup caches for our build, and further use cached versions of dependencies when installing. This is less of a problem now that the repo is public, but still. (EDIT: now under 4 minutes, not bad.)
@threepointone threepointone self-assigned this Nov 14, 2020
@threepointone threepointone mentioned this issue Nov 14, 2020
20 tasks
@threepointone threepointone changed the title export modular-scripts/widgets.macro export modular-views.macro Nov 15, 2020
threepointone added a commit that referenced this issue Nov 17, 2020
This implements the view map as a babel macro, replacing app/views.ts from our previous iteration. This is written in plain javascript because it's otherwise hard to author and test the macro. In a future revision, I'll add jsdoc annotations
(https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html)

I also took this opportunity to delete docs/intro.md.

Fixes #59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants