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 script to install packages locally in a DXP checkout #98

Merged
merged 1 commit into from
Oct 1, 2020

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Oct 1, 2020

This is a crude script that will almost certainly have to be evolved as the shape and structure of the monorepo evolves, but it works for now with the set of packages that we currently have.

It addresses a bunch of "gotchas" that come with trying to test out packages with local changes in a liferay-portal checkout. Depending on which packages you want to test and the dependency graph between them, you may find that yarn link (or even yarn add) doesn't do what you want. Even if you manually copy files (cp), things still might not work because Gradle may decide to re-run a task that ends up blowing away any changes you make locally under node_modules.

At least for the simple case, this script was good enough to allow me to test #97, even though it involved changes to both eslint-config and npm-scripts.

As you can see, it does three things:

  • Blows away nested node_modules folders inside the monorepo, because yarn add can end up copying these over in ways that cause confusion (broken bin links and so on).
  • Cleans out the existing versions of the dependencies (in my testing, removing and then re-adding seemed to produce a cleaner result than just overwriting).
  • Adds the modules to be tested.

Like I said at the start, this is a hack (and not even trying to make it work on Windows), but it's a useful hack.

This is a crude script that will almost certainly have to be evolved as
the shape and structure of the monorepo evolves, but it works for now
with the set of packages that we currently have.

It addresses a bunch of "gotchas" that come with trying to test out
packages with local changes in a liferay-portal checkout. Depending on
which packages you want to test and the dependency graph between them,
you may find that `yarn link` (or even `yarn add`) doesn't do what you
want. Even if you manually copy files (`cp`), things still might not
work because Gradle may decide to re-run a task that ends up blowing
away any changes you make locally under `node_modules`.

At least for the simple case, this script was good enough to allow me to
test:

    #97

even though it involved changes to both `eslint-config` and
`npm-scripts`.

As you can see, it does three things:

- Blows away nested `node_modules` folders inside the monorepo, because
  `yarn add` can end up copying these over in ways that cause confusion
  (broken `bin` links and so on).
- Cleans out the existing versions of the dependencies (in my testing,
  removing and then re-adding seemed to produce a cleaner result than
  just overwriting).
- Adds the modules to be tested.

Like I said at the start, this is a hack (and not even trying to make it
work on Windows), but it's a useful hack.
@wincent wincent added the feat label Oct 1, 2020
@wincent
Copy link
Contributor Author

wincent commented Oct 1, 2020

cc: resident #BashGuy, @julien

hacker


Fun fact: the high-quality version of that GIF on Giphy is 33 MB.

@jbalsas
Copy link
Contributor

jbalsas commented Oct 1, 2020

Need any more duct tape?

ducttape

@wincent
Copy link
Contributor Author

wincent commented Oct 1, 2020

Need any more duct tape?

Not a real review.

judgment

@jbalsas
Copy link
Contributor

jbalsas commented Oct 1, 2020

Need any more duct tape?

Not a real review.

Review this

reviewthis

@wincent
Copy link
Contributor Author

wincent commented Oct 1, 2020

over-the-line

So, I guess I'm just going to merge this wonderful code.

@wincent wincent merged commit 023ef9e into master Oct 1, 2020
@wincent wincent deleted the wincent/install branch October 1, 2020 14:41
@jbalsas
Copy link
Contributor

jbalsas commented Oct 1, 2020

LGTM! 😂

wincent added a commit that referenced this pull request Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants