-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Expo 52 + Full typescript #207
Open
natew
wants to merge
4
commits into
main
Choose a base branch
from
full-typescript
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🚅 Deployed to the one-pr-207 environment in onestack.dev
|
Ok I have cooked - this plugin works: {
name: 'one-node-module-typescript',
transform: {
order: 'pre',
async handler(code: string, id: string) {
if (!/\.tsx?$/.test(id)) return
if (!id.includes('node_modules')) {
return
}
// we need to keep fake objects for type exports
const typeExportsMatch = code.match(/^\s*export\s+type\s+([^\s]+)/gi)
let output =
(
await swcTransform(id, code, {
mode: mode === 'dev' ? 'serve' : 'build',
})
)?.code || ''
// add back in export types as fake objects:
if (typeExportsMatch) {
for (const typeExport of Array.from(typeExportsMatch)) {
const fakeExport = `${typeExport.replace(' type ', ' const ')} = {};`
output += `\n${fakeExport}\n`
}
}
return output
},
},
}, For now at least, just use SWC, but add back in type exports as const exports. Hacky yes but should work for 99% of use cases for now and we can explore a robust solution in the meantime. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ok so went into this a bit, Expo is just publishing typescript directly in
expo-modules-core
, and not using type import/exports, so you can't just strip types. Rollup doesn't allow importing things that don't exist. One potential workaround would be to have some sort of custom thing that turns type exports into empty objects, but idk that seems hacky even if it would be significantly faster. Since this only will happen by people who are publishing typescript directly, its rare so we can probably do the "right" thing and use the official rollup typescript plugin.I added the rollup typescript plugin as a test. It didn't work off the top so I added logs:
It does properly hit
loading
andgogog
, but infinal
what happens is:So what we'll need to do is probably fork (eventually contribute back potentially) to enable supporting tsconfig.json that is inside a node_module. I think it shouldn't be too hard - basically just need to refactor the logic around loading the tsconfig so that: