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

feat: add dir into deps #2

Closed
wants to merge 44 commits into from
Closed

feat: add dir into deps #2

wants to merge 44 commits into from

Conversation

beyondkmp
Copy link
Owner

No description provided.

@beyondkmp beyondkmp closed this Apr 2, 2024
@beyondkmp beyondkmp reopened this Apr 2, 2024

if (info.conflictDependency) {
for (const dep of info.conflictDependency) {
const source = platformPackager.info.appDir + path.sep + "node_modules" + path.sep + info.name + path.sep + "node_modules" + path.sep + dep.name
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, how do we know that it's a double-nested node_modules? I'm worried about the hard-coded approach here, but wanted to understand the context here.

Copy link
Owner Author

@beyondkmp beyondkmp Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All dependencies are placed directly into a map. If it's found that there's already a dependency with the same name but a different version in the map, it indicates a conflict. Currently, I'm placing the conflicting version directly into the node_modules of the current package,so we know that it's a double-nested node_modules.

https://github.com/beyondkmp/app-builder/blob/c85c25c911b6f5fc508afbbbd705b37220571b12/pkg/node-modules/nodeModuleCollector.go#L91-L107

The name conflictDependency might be somewhat misleading; we could consider renaming it.

@@ -3,7 +3,7 @@ import { executeAppBuilderAsJson } from "./appBuilder"

export function createLazyProductionDeps(projectDir: string, excludedDependencies: Array<string> | null) {
return new Lazy(async () => {
const args = ["node-dep-tree", "--dir", projectDir]
const args = ["node-dep-tree", "--flatten", "--dir", projectDir]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a new arg --flatten you've implemented in app-builder or something that already exists? Curious as to what changes you made to app-builder for this to work? 🙂

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to disrupt the original architecture, so I added this new parameter. Only with this parameter will the content be output in the new format; without it, the content will remain in the old format.

https://github.com/beyondkmp/app-builder/blob/c85c25c911b6f5fc508afbbbd705b37220571b12/pkg/node-modules/tree.go#L47-L48

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! Love it

@beyondkmp beyondkmp closed this May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants