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

Unboxing Blueprint4 video: Upgrading from BP3 -> BP4 and sharing thoughts. #5225

Open
vogievetsky opened this issue Apr 5, 2022 · 2 comments

Comments

@vogievetsky
Copy link

Writing on behalf of myself and the Apache Druid project, I just wanted to say THANK YOU to the blueprint team for the excellent library and all the love y'all pour into it.

Recently @jgoz and I upgraded the Blueprint version used by the Druid web console from 3 to 4 and recorded the experience, along with fun memories from the early days of using Blueprint.

We decided to do it sort of like an "unboxing video" that we would do whenever version 4 was released and it happened to coincide with the anniversary of the time we updated Druid to use BP3 which was cool.

The upgrade itself went smoothly thanks to all the work that was put into version 3 to add depreciation messages and such. I was afraid that the video is going to be totally boring until we ran into one little issue concerning the color overrides (the Druid console overrides the BP colors). The issue we encountered was that ./node_modules/@blueprintjs/core/src/common/_mixins.scss imports color definitions from the "lib" folder in @blueprintjs/colors but we need to import it from the "src" folder. The "src" version includes !default in variable definitions, which allows us to override color variables, but the "lib" version does not.

You can see us struggle with this in the video or look at the diff in webpack.config.js to see how we solved it. I am not sure what to make of it... is it a bug or are we stepping outside of the bounds of how BP should be used? In any-case we had a great time and maybe this video can help guide someone else in a similar position.

Thank you again for BP4, I heartily look forward to making another video of us upgrading to BP5 when it is time!

@adidahiya
Copy link
Contributor

adidahiya commented Apr 13, 2022

@vogievetsky thanks for the kind words and sharing your experience working with Blueprint. After spending a lot of time working through this upgrade in our internal code bases, it’s really helpful for me to see how BP4 has landed for dev teams outside of Palantir. There’s often room for improvement in the migration paths for Blueprint upgrades, and you’ve identified some of it here. Here are my findings after reviewing your unboxing video:

  • The lack of ! default modifiers for variables in the @blueprintjs/colors/lib/scss/colors.scss file is a bug, I can fix that. Note that the /lib/ folders contain the public API, and you should reach for imports from there first. I'm aware that some Blueprint users customize the color variables and re-compile the default styles with Sass imports reaching into our /src/ folders, but keep in mind that, at least for now, this is a semi-private API and we aren't guaranteeing backwards compatibility for the syntax in Blueprint Sass source files. See Compilation issue with newer version of SASS #4032 (comment) for more info. This will be the case until we have a more fleshed-out theming API.
  • On a related note, I realized that the creation of the @blueprintjs/colors package did not come with much guidance or documentation about when to use it. I'll add documentation about this new package.
  • I'll have to look into why the number of NPM packages installed increased so much in BP4. I think it was due to the addition of the change-case package. It might be worth dropping this import and just implementing some simple string manipulation functionality in our code base to reduce our dependency weight.
  • Regarding the CSS class name prefix ("bp3-", "bp4-"), there are a few issues/notes:
    • Overriding the $bp-ns or $ns variable defaults should not be allowed for Sass consumers. Changing this is meaningless, since the JS implementation of the components has the namespace hardcoded to "bp3-" or "bp4-" or whatever, and unless we also provide an API to customize this in JS, it shouldn't be allowed in CSS. I'll have to remove the ! default modifier from these variables.
    • This class name prefix will continue to change across major versions of Blueprint. We added it a while back so that we could have elements with different Blueprint versions rendered on the same page without conflict. We knew that this would create some friction for developers, so we added JS/Sass/Less variables for the current namespace to the public API. The last piece of tooling which makes this namespace easier to work with is our ESLint and Stylelint plugins: @blueprintjs/eslint-plugin and @blueprintjs/stylelint-plugin. We use these extensively inside Palantir and they've been really helpful in the migration, as well as future-proofing for future migrations. I admit I haven't done a great job advertising / documenting these for open source users, so I'll make an effort to do that better (at least adding them to the migration guides!).

Thanks again for the feedback, and keep it coming please! 👍🏽

@jgoz
Copy link

jgoz commented Apr 13, 2022

I'll have to look into why the number of NPM packages installed increased so much in BP4. I think it was due to the addition of the change-case package. It might be worth dropping this import and just implementing some simple string manipulation functionality in our code base to reduce our dependency weight.

Yeah, I'm pretty sure change-case is the reason. There is one package for each kind of case change you might want to make and change-case is the root of that dependency tree. From what I recall, you can depend on only the change operation packages you actually need, but an internal helper for this kind of thing sounds good too.

The last piece of tooling which makes this namespace easier to work with is our ESLint and Stylelint plugins: @blueprintjs/eslint-plugin and @blueprintjs/stylelint-plugin.

Ah, great tip, thank you! We will adopt those and include that in the BP5 video 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants