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

fix(crwa): Fixes telemetry issues on CRWA #6434

Merged
merged 5 commits into from
Sep 21, 2022

Conversation

dac09
Copy link
Collaborator

@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
packages/telemetry/README.md Outdated Show resolved Hide resolved
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
@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
Co-authored-by: David Thyresson <dthyresson@gmail.com>
jtoar pushed a commit that referenced this pull request Sep 24, 2022
Co-authored-by: David Thyresson <dthyresson@gmail.com>
jtoar pushed a commit that referenced this pull request Sep 24, 2022
Co-authored-by: David Thyresson <dthyresson@gmail.com>
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
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants