-
Notifications
You must be signed in to change notification settings - Fork 142
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
Support Node 16 #965
Merged
+184
−329
Merged
Support Node 16 #965
Changes from 1 commit
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
298e638
add explicit dependency on ws
raiyaj 906b23e
add custom docs url for 403 errors
raiyaj b8d680d
remove performance timer
raiyaj fa17a85
add explicit support for node 16
raiyaj 86f9353
add explicit pwa-kit-dev support for node 16
raiyaj 3eb9ee1
add explicit support for node 16 across the board
raiyaj c9e24b6
delete performance timer tests
raiyaj e81ea18
add explicit support for node 18
raiyaj 74081e3
add node 16 and 18 to test matrix
raiyaj 4b463a5
Revert "add explicit support for node 18"
raiyaj 5c23e0e
remove 18 from test matrix
raiyaj 261d49a
fix use-product-view-modal test
raiyaj 2e88658
fix use-wishlist test
raiyaj 6684628
lint
raiyaj 5678a28
fix product-view test
raiyaj 4f3dd08
lint
raiyaj 614549f
try to fix with-registration test
raiyaj 5182c84
update changelog
raiyaj 60ae753
Merge branch 'develop' into pwa-kit-runtime-node16
raiyaj 3c2e5a4
update pwa-kit-dev changelog
raiyaj ea563c5
add node 16 to windows test matrix
raiyaj 40be1e2
change default pwa kit version to 16.x
raiyaj 942dad5
try to fix flaky reset-password test
raiyaj cdbe418
switch generated builds to node 16
raiyaj fdc6d21
add npm 7 and 8 to windows test matrix
raiyaj 70e1bdf
try to make reset-password test even more resilient
raiyaj 7a7567a
add -y flag to npx command to prevent timeouts when generating projec…
raiyaj 58f8ca4
try to make registration test more resilient
raiyaj 35dad46
try to make account test more resilient
raiyaj b571ee7
try to make registration test more resilient
raiyaj bdf0896
conditionally pass -y flag to npx based on current node/npm version
raiyaj f73e739
exclude unnecessary node/npm combinations from test matrices
raiyaj 8078e1b
fix indentation
raiyaj 25dc0d6
make sure pwa-kit-create-app@latest gets installed
raiyaj 5674fe6
Merge branch 'develop' into pwa-kit-runtime-node16
raiyaj 0f2cf9d
set npx flags based on the version of npm, not node
raiyaj 1298a0f
update IS_DEFAULT_NPM
raiyaj f123067
try to make reset-password test more resilient
raiyaj d6e14c1
clean up tests
raiyaj 3eb363b
fix sdk dir path
raiyaj d90306f
add logs
raiyaj 85fbe74
make sure sdkDir resolves to pwa-kit-dev
raiyaj 5c7552a
refactor
raiyaj 0dcfef7
return a default path rather than throwing an error
raiyaj e7f7df6
Merge branch 'develop' into pwa-kit-runtime-node16
raiyaj f230864
update READMEs and babel configs
raiyaj 0c50c68
try to make tests more resilient
raiyaj ee119ad
lint
raiyaj 0e6e00e
add comment about looking for pwa-kit-dev node_modules in two places
raiyaj b7186e8
try to make tests more resilient
raiyaj 38dbed1
undo unnecessary change
raiyaj 5bc8c36
ensure pwa-kit publish steps are only run once
raiyaj 570c53d
Merge branch 'develop' into pwa-kit-runtime-node16
raiyaj 87cf7eb
remove node 16 / npm 6 builds
raiyaj cba5cb5
try to make reset-password test more resilient
raiyaj 85cd2dc
Merge branch 'develop' into pwa-kit-runtime-node16
raiyaj c08035f
try to make registration test more resilient
raiyaj 6ec6d7d
Merge branch 'develop' into pwa-kit-runtime-node16
raiyaj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
return a default path rather than throwing an error
- Loading branch information
commit 0dcfef7f4044ba32e09ba16def9f6ba3b66011f9
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Good finding @raiyaj!
It looks like we don't need the last one since we saw that generates the wrong path with the duplicated
node_modules
folder.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.
Thanks Adam! I was thinking of removing it initially, but I added some logging here (in the Setup Ubuntu Machine step), and saw that when a project is generated in CI,
__dirname
is/home/runner/work/pwa-kit/pwa-kit/packages/pwa-kit-dev/dist/configs/webpack
, whereas locally it's/Users/rjessa/generated-test-project/node_modules/pwa-kit-dev/configs/webpack
, so we need to try both options.I copied the
candidates
idea from here, where we do something similar.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.
The method's name
findInProjectThenSDK
implies two places but we now look at three places.It can be confusing for anyone else looking at the logic or facing a similar problem in the future.
@raiyaj Do you mind leaving a comment in the code explaining why we need to do this to support NPM 7 and Node 16?