Skip to content

Commit

Permalink
refactor: update dependencies + self host repository. (#132)
Browse files Browse the repository at this point in the history
* 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.)
  • Loading branch information
threepointone authored Nov 12, 2020
1 parent 8e84b1b commit 1b34e70
Show file tree
Hide file tree
Showing 80 changed files with 3,757 additions and 7,965 deletions.
6 changes: 6 additions & 0 deletions .changeset/fifty-kings-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'eslint-config-modular-app': minor
'modular-scripts': minor
---

Update craco and create-react-app
7 changes: 7 additions & 0 deletions .changeset/young-dragons-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'create-modular-react-app': minor
'eslint-config-modular-app': minor
'modular-scripts': minor
---

refactor the repository to become a modular project itself.
5 changes: 4 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ module.exports = {
'import/first': ERROR,
'import/no-amd': ERROR,
'import/no-anonymous-default-export': WARN,
'import/no-extraneous-dependencies': ERROR,
// disabling this because it doesn't consider local yarn workspaces
// or root dependencies as legitimate, and we use it extensively.
// 'import/no-extraneous-dependencies': ERROR,
'import/no-webpack-loader-syntax': ERROR,
},
settings: {
Expand Down Expand Up @@ -80,6 +82,7 @@ module.exports = {
rules: {
// TypeScript compilation already ensures that default imports exist in the referenced module
'import/default': OFF,
'@typescript-eslint/ban-ts-comment': OFF,
},
},
{
Expand Down
34 changes: 0 additions & 34 deletions .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,37 +62,3 @@ jobs:
v2-${{ runner.os }}-yarn-
- run: yarn
- run: yarn test
test_e2e:
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [12.x]

steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}
- name: Get Yarn cache directory path
id: yarn-cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"
- name: Cache Yarn
uses: actions/cache@v2
id: yarn-cache
with:
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
key: v2-${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
v2-${{ runner.os }}-yarn-
- name: Install dependencies
run: yarn
env:
PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: 'true'
- name: Run E2E tests
uses: ianwalter/puppeteer-container@v4.0.0
with:
args: yarn e2e --forceExit
env:
CI: 'true'
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
# Dependencies
node_modules

packages/sample-app
packages/sample-view
packages/sample-package


# Build output
/packages/*/build/

Expand Down
4 changes: 2 additions & 2 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ const pkgJson = require('./package.json');
const supportedNodeVersion = semver.minVersion(pkgJson.engines.node).version;

module.exports = (api) => {
const env = api.env();
api.cache(true);

return {
presets: [
[
'@babel/preset-env',
{
targets: {
node: env === 'test' ? 'current' : supportedNodeVersion,
node: supportedNodeVersion,
},
},
],
Expand Down
Loading

0 comments on commit 1b34e70

Please sign in to comment.