-
Notifications
You must be signed in to change notification settings - Fork 124
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
chore(deps): update semver ranges for @stencil/core package #318
Conversation
@@ -15,7 +15,7 @@ | |||
}, | |||
"devDependencies": { | |||
"@ionic/prettier-config": "^2.0.0", | |||
"@stencil/core": "^2.9.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think NPM knows how to bump packages with text in their version string. e.g. I'm not sure if it knows that v3.0.0-rc.0
(the next type of release we'll do) is a higher version than v3.0.0-beta.0
. But maybe I'm wrong there - do you know if that's the case?
Ideally, I don't want to have to put up a PR when we go from beta->rc->ga, mostly so we don't have as tight of a coordination schedule when we're rolling v3.0.0 out. In the event that NPM doesn't know that rc comes after beta (and that 3.0.0
comes after rc), I'm not sure what the best course of action is here. IIRC npm can get finicky here. Maybe using v3-next
in our accepted versions string in addition to ^3.0.0
? I'm not 100% sure npm will accept that without doing something like:
- Installing v3-next with
npm i @stencil/core@v3-next
- Manually edit the dependencies
- Installing the latest Stencil Core (on v2) with
npm i @stencil/core@latest
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not 100% sure on some of these range matching bits, but I went ahead with this range after checking here: https://jubianchi.github.io/semver-check/#/^3.0.0-beta.0/3.0.0
and doing some reading of the semver
package documentation, here: https://github.com/npm/node-semver
I believe that, for instance, 3.0.0
or 3.0.1
will be accepted as meeting ^3.0.0-beta.0
(as will, I think, 3.0.0-beta.1
, 3.0.0-rc.0
, etc).
@@ -15,7 +15,7 @@ | |||
}, | |||
"devDependencies": { | |||
"@ionic/prettier-config": "^2.0.0", | |||
"@stencil/core": "^2.9.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update the package-lock.json
files here (and in the other files) as well? devDependencies
and peerDependencies
still get written to the manifest when running npm i
. I think this might get a little tricky with respect to my other comment on this line though 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah that's probably a good idea 😅
I think I missed it just because I was lerna
-ing and I thought it would do that automatically!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 Things:
- I had to update Stencil Sass as well when testing this. I didn't think we'd need to update that, but I was wrong it appears.
I'll put up the PR in a bit to fix that.(chore(deps): support stencil v3 sass#198) - For the example projects, it looks like there's a lot of changes in the
component-library
package-lock, some in the vue one, and none for angular/react. Do we need any of them? Do we need to update Angular & React (or were they up to date?)?
For the |
Ah, I suspect that these haven't been updated in that case :-( Well, I suppose we need to update at some point, might as well do it now 🚢 |
9bc1384
to
4202f36
Compare
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build
) was run locally for affected output targetsnpm test
) were run locally and passednpm run prettier
) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue URL:
What is the new behavior?
This change updates the semver ranges for the
@stencil/core
peer dependency for various framework wrapper packages. This should let us use them with stencil v3.Does this introduce a breaking change?
testing
I think this all works, but to be honest I am not the biggest expert on this repo.
Here's what I did to check that it's working.
building output targets
First we need to build and package test versions of the framework wrapper packages which have the new version.
This will bootstrap the repo, run build in all the packages, and then run
npm pack
andnpm link
in each of them (I'm not super familiar with lerna so very probably there is a better way to do this!):After that we should have a
.tgz
file for each built and set up so that we can install it on the framework side.install in framework
Ok so basically we'll need to install the packages we just built in framework and install Stencil v3.
First we need to link the framework wrapper packages we just built. Clone the framework repo and in the root of it do the following:
cd core npm i npm link @stencil/angular-output-target --save npm link @stencil/react-output-target --save npm link @stencil/vue-output-target --save
You may not need the
--save
flag - I ran into a confusing interaction betweennpm
,lerna
, andvolta
and this was the way that I got it to work 🤷♀️Once you have the framework wrapper packages installed then you'll want to install the v3 beta of Stencil, like so:
You should still see some peer dependency errors, but they should only be from
@stencil/sass
and not from@stencil/angular-output-target
and company.Ok finally we can run a build in Framework:
build and test react, angular, and vue packages in framework
Now we need to test that we can build and test the framework-specific packages for the Ionic framework. We can lean on Lerna for a bunch of this.
First I had to do this to get package dependencies to install correctly:
then we can build all the packages by doing
🚨⚠️ NOTE⚠️ 🚨 at present the build does not work because of a known issue with circular type exports. However, repeating the above steps with
@stencil/[email protected]
installed does work.