-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Try aliasing preact typescript declaration to fix ts errors #11088
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
Try aliasing preact typescript declaration to fix ts errors #11088
Conversation
851fd44
to
38e4b97
Compare
nvm CI sucks I'll stay on this branch. |
38e4b97
to
470cc40
Compare
walk(filePath); | ||
} else { | ||
if (filePath.endsWith('.d.ts')) { | ||
const content = fs.readFileSync(filePath, 'utf8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition
const content = fs.readFileSync(filePath, 'utf8'); | ||
if (preactImportRegex.test(content)) { | ||
const newContent = content.replace(preactImportRegex, '// replaced import from preact'); | ||
fs.writeFileSync(filePath, snippet + newContent, 'utf8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition
const content = fs.readFileSync(filePath, 'utf8'); | ||
const capture = preactImportRegex.exec(content); | ||
if (capture) { | ||
const groups = capture[1].split(',').map(s => s.trim()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shrewd readers would notice that this breaks completely with multiline imports
import {
x,
y,
z,
} from 'preact';
I know, but we can cross that bridge when we come to it - prefer to get this out to unblock asap.
attempting to unblock #10590
Right now the feedback PR fails ts3.8 e2e tests: https://github.com/getsentry/sentry-javascript/actions/runs/8268834386/job/22630054663?pr=10590
This is because of TS issues, preact types do not work well with older TS types:
Essentially these types don't work on older versions and because the feedback package is depended on by the browser package, we will break a certain set of our users. We have to fix this before we can ship because of the browser package dependency.
To get around this, we do a workaround with
packages/feedback/scripts/shim-preact-export.js
. Essentially we shim the preact type in our exports, which should change what the declaration files themselves point to. So something like the following:turns into