Skip to content

Conversation

@dac09
Copy link
Contributor

@dac09 dac09 commented Sep 21, 2022

Diagnosis of the issue

The telmetry script was failing, because it couldn't find the module typescript.

You can see this by running with the verbose flag: REDWOOD_VERBOSE_TELEMETRY=true yarn create-redwood-app bazinga

Changes

  1. typescript is an explicit dependency of rwjs/internal . This is because we use it for detecting strict mode.

What was happening was this import sequence:
crwa -> rwjs/structure -> rwjs/internal -> ts

Why is TS even imported here? Because of the import statement in rwjs/structure project.ts and host.ts:

import { getPaths, processPagesDir } from '@redwoodjs/internal'
  1. Makes sure structure also imports from specific files in internal i.e.
-import { getPaths, processPagesDir } from '@redwoodjs/internal'
+import { getPaths, processPagesDir } from '@redwoodjs/internal/dist/paths'
  1. Enables the eslint rule to prevent direct imports in structure too
    (as a result, the formatting, etc. happened too)

@dac09 dac09 added the release:fix This PR is a fix label Sep 21, 2022
@dthyresson
Copy link
Contributor

Did a review of the code and LGTM from what I understand.

However, the telemetry package README is a bit scant:

https://github.com/redwoodjs/redwood/blob/main/packages/telemetry/README.md

Maybe add the description above as well as some of the helpful enviers for debugging?

@dac09 dac09 enabled auto-merge (squash) September 21, 2022 14:12
@dac09 dac09 merged commit 7b3665d into redwoodjs:main Sep 21, 2022
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Sep 21, 2022
@dac09 dac09 deleted the fix/telemetry-module-err branch September 21, 2022 16:27
jtoar pushed a commit that referenced this pull request Sep 24, 2022
jtoar pushed a commit that referenced this pull request Sep 24, 2022
jtoar pushed a commit that referenced this pull request Sep 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:fix This PR is a fix

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants