fix: cli should rewrite the aliases #847
Conversation
facebook-github-bot
added
the
CLA Signed
workflow: benchmarks/sizeComparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.
|
| @@ -24,7 +24,7 @@ export type CliConfig = { | |||
| babelPluginsPost?: $ReadOnlyArray<any>, | |||
| modules_EXPERIMENTAL: $ReadOnlyArray<ModuleType>, | |||
| useCSSLayers?: boolean, | |||
| styleXConfig?: { +[string]: mixed }, | |||
| styleXConfig?: StyleXOptions, | |||
packages/cli/src/plugins.js
Outdated
| const matchesFileSuffix = (allowedSuffix: string) => (filename: string) => | ||
| filename.endsWith(`${allowedSuffix}.js`) || | ||
| filename.endsWith(`${allowedSuffix}.ts`) || | ||
| filename.endsWith(`${allowedSuffix}.tsx`) || | ||
| filename.endsWith(`${allowedSuffix}.jsx`) || | ||
| filename.endsWith(`${allowedSuffix}.mjs`) || | ||
| filename.endsWith(`${allowedSuffix}.cjs`) || | ||
| filename.endsWith(allowedSuffix); | ||
|
|
||
| const EXTENSIONS = ['.js', '.ts', '.tsx', '.jsx', '.mjs', '.cjs']; | ||
| const filePathResolver = ( | ||
| relativeFilePath: string, | ||
| sourceFilePath: string, | ||
| aliases: $ReadOnly<{ [string]: $ReadOnlyArray<string> }>, | ||
| ): ?string => { | ||
| // Try importing without adding any extension | ||
| // and then every supported extension | ||
| for (const ext of ['', ...EXTENSIONS]) { | ||
| const importPathStr = relativeFilePath + ext; | ||
|
|
||
| // Try to resolve relative paths as is | ||
| if (importPathStr.startsWith('.')) { | ||
| try { | ||
| return moduleResolve(importPathStr, url.pathToFileURL(sourceFilePath)) | ||
| .pathname; | ||
| } catch { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Otherwise, try to resolve the path with aliases | ||
| const allAliases = possibleAliasedPaths(importPathStr, aliases); | ||
| for (const possiblePath of allAliases) { | ||
| try { | ||
| return moduleResolve(possiblePath, url.pathToFileURL(sourceFilePath)) | ||
| .pathname; | ||
| } catch { | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
| // Failed to resolve the file path | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
Did you intend to use the newly exported functions from the babel plugin here, or is there a special edge case that makes them incompatible?
There was a problem hiding this comment.
The Babel plugin is compiled into a single bundle which makes using the individual exports impossible. I will need to refactor some of these utility functions to a separate package.
For now I just copied the relevant bits.
packages/cli/src/plugins.js
Outdated
| const filePathResolver = ( | ||
| relativeFilePath: string, | ||
| sourceFilePath: string, | ||
| aliases: $ReadOnly<{ [string]: $ReadOnlyArray<string> }>, |
There was a problem hiding this comment.
Are the aliases in here going to have to be something you specify in the stylex.json where you have to explicitly tell the CLI where the aliases resolve to?
There was a problem hiding this comment.
Yes. This is already the case in the cli-example.
There was a problem hiding this comment.
However, there is a PR up right now which discovers aliases automatically. That PR will need to be updated to do that work in the CLI as well.
It might also make sense to move alias rewriting to the main StyleX plugin as it already has all the relevant logic. (This might be the right thing to do actually....)
packages/cli/src/plugins.js
Outdated
| }; | ||
|
|
||
| const matchesFileSuffix = (allowedSuffix: string) => (filename: string) => | ||
| filename.endsWith(`${allowedSuffix}.js`) || |
There was a problem hiding this comment.
can we use a loop ie. EXTENSIONS.some(ext => filename.endsWith(${allowedSuffix}${ext}))
nmn
force-pushed
the
fix/cli-rewrite-aliases
branch
from
7e3b37c to
3ac5d73
Compare
What changed / motivation ?
The CLI will now rewrite imports based on the
aliasesthat are passed intostylexConfig.Resolves #846
Side note:
It might make sense to extract some of the utilities that were copied from the
babel-pluginto a separateutilspackage for such use-cases.