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

Enable local consumption of OUI #813

Merged
merged 2 commits into from
Jun 19, 2023

Conversation

AMoo-Miki
Copy link
Collaborator

@AMoo-Miki AMoo-Miki commented Jun 19, 2023

Description

This change allows OUI to self-clean while being installed during development when linked to as a dependency.

Also:

  • Fixed linting of sass files

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • All tests pass
    • yarn lint
    • yarn test-unit
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

scripts/compile-oui.js Outdated Show resolved Hide resolved
.sass-lint.yml Show resolved Hide resolved
With the upgrade os `eslint`, the location of the default formatter changed. This is just a hack to work around the problem in the now abandoned `sass-lint`.

Signed-off-by: Miki <miki@amazon.com>
@@ -14,7 +14,7 @@ packages/eslint-plugin
types/

# ignore everything in `scripts` except postinstall.js
scripts/!(postinstall.js)
scripts/!(postinstall.js|preinstall.js)
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we update the comment here to include preinstall as well?


// Only run when installed as a dep
if (!INIT_CWD?.startsWith?.(PWD)) {
const depsToKeep = [
Copy link
Member

Choose a reason for hiding this comment

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

Just curious to know how did we determine this list of deps and know we haven't missed any?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Left a comment in the code:

  /* These are deps and types which get installed when a production package is installed.
   * When this library is linked as a dep to another project, having all the deps could
   * confuse or conflict the project's compilers.
   *
   * When being installed as dep of another project in production, `node_modules` would
   * be empty.
   */

'docs',
'generator-oui',
'packages/eslint-plugin',
'packages/react-datepicker',
Copy link
Member

Choose a reason for hiding this comment

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

why this one?

Copy link
Collaborator Author

@AMoo-Miki AMoo-Miki Jun 19, 2023

Choose a reason for hiding this comment

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

We never use the content of packages/react-datepicker; it is only the built artifact, packages/react-datepicker.js that we package and consume.

@manasvinibs
Copy link
Member

Is there a cmd to run during development to get this script triggered?

Signed-off-by: Miki <miki@amazon.com>
@AMoo-Miki
Copy link
Collaborator Author

Is there a cmd to run during development to get this script triggered?

In the package.json of the project that wants to use this, instead of the version pattern, file:<absolute path> would be used:
"@opensearch-project/oui": "file:/Users/miki/projects/oui"

@AMoo-Miki AMoo-Miki merged commit 2c26b04 into opensearch-project:main Jun 19, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 19, 2023
* Fix linting sass files

With the upgrade os `eslint`, the location of the default formatter changed. This is just a hack to work around the problem in the now abandoned `sass-lint`.

Signed-off-by: Miki <miki@amazon.com>

* Add preinstall script

Signed-off-by: Miki <miki@amazon.com>

---------

Signed-off-by: Miki <miki@amazon.com>
(cherry picked from commit 2c26b04)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
BSFishy pushed a commit that referenced this pull request Jun 20, 2023
* Fix linting sass files

With the upgrade os `eslint`, the location of the default formatter changed. This is just a hack to work around the problem in the now abandoned `sass-lint`.



* Add preinstall script



---------


(cherry picked from commit 2c26b04)

Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@AMoo-Miki AMoo-Miki deleted the enable-local-dev branch September 27, 2023 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants