feat: web compatibility W-20175875 #1650
Conversation
| import { SfError } from '@salesforce/core/sfError'; | ||
| import { ensureArray } from '@salesforce/kit'; | ||
| import { ComponentLike, SourceComponent } from '../resolve'; | ||
| import { SourceComponentWithContent, SourceComponent } from '../resolve/sourceComponent'; |
There was a problem hiding this comment.
SourceComponentWithContent passed the rule of 3
There was a problem hiding this comment.
What's the rule of 3 in this context?
There was a problem hiding this comment.
if the same code is in 3 places, make it shared/reusable
There was a problem hiding this comment.
so now there's SouceComponentWithContent instead of several places making SourceComponent & {content: string}
| (filePath: string): string => { | ||
| // Normalize paths to ensure relative() works correctly on Windows | ||
| const normalizedContent = component.content.split(sep).join(posix.sep); | ||
| const normalizedFilePath = filePath.split(sep).join(posix.sep); |
There was a problem hiding this comment.
took me a while of playing with it, but since both of these are relative, and then posix.relative is between them, then I think it should be ok even without cwd?
There was a problem hiding this comment.
That sounds right to me?
| */ | ||
| import { SourceComponent, SourceComponentWithContent } from '../resolve/sourceComponent'; | ||
|
|
||
| export const isWebAppBundle = (component: SourceComponent): component is SourceComponentWithContent => |
There was a problem hiding this comment.
was repeated in a few places. made it a TS guard to help stuff downstream of the guard not have to barbarically assert content!
jfeingold35
left a comment
jfeingold35
left a comment
There was a problem hiding this comment.
Is this something that we can add test coverage for?
| import { SfError } from '@salesforce/core/sfError'; | ||
| import { ensureArray } from '@salesforce/kit'; | ||
| import { ComponentLike, SourceComponent } from '../resolve'; | ||
| import { SourceComponentWithContent, SourceComponent } from '../resolve/sourceComponent'; |
There was a problem hiding this comment.
What's the rule of 3 in this context?
| (filePath: string): string => { | ||
| // Normalize paths to ensure relative() works correctly on Windows | ||
| const normalizedContent = component.content.split(sep).join(posix.sep); | ||
| const normalizedFilePath = filePath.split(sep).join(posix.sep); |
There was a problem hiding this comment.
That sounds right to me?
What does this PR do?
by default, getFileResponses makes the file paths relative to cwd.
in extensions,
cwdis always/by default (current extensions manipulate it via chdir but that's not going to happen on the web)if the CompSet has a projectDirectory set, and it's different from cwd, make it relative to that.
//note: when we do the major version of SDR, I'd LOVE to have the DeployResult be an immutable object including fileResponses, calculated once, instead of a class with a method you call to calculate them (and then see if anyone's using the other public properties or if some are only there to be able to calculate the fileResponses.
What issues does this PR fix or reference?
@W-20175875@