feat(extensions): handle build step for expressjs framework - #1315
Open
canon-cat wants to merge 4 commits into
Open
feat(extensions): handle build step for expressjs framework#1315canon-cat wants to merge 4 commits into
canon-cat wants to merge 4 commits into
Conversation
asanvaq
requested changes
Aug 2, 2026
asanvaq
left a comment
asanvaq
left a comment
Collaborator
There was a problem hiding this comment.
Thanks! From the documentation side, I have added a few comments.
| It facilitates the installation of Express application dependencies, including | ||
| Node.js and npm, inside the rock. Additionally, it transfers your project files | ||
| to ``/app`` within the rock. | ||
| Node.js and npm, inside the rock. Extension discovers location of the |
Collaborator
There was a problem hiding this comment.
Suggested change
| Node.js and npm, inside the rock. Extension discovers location of the | |
| Node.js and npm, inside the rock. The extension discovers the location of the |
| Node.js and npm, inside the rock. Additionally, it transfers your project files | ||
| to ``/app`` within the rock. | ||
| Node.js and npm, inside the rock. Extension discovers location of the | ||
| ``package.json``, but can only package a single application. If application |
Collaborator
There was a problem hiding this comment.
Suggested change
| ``package.json``, but can only package a single application. If application | |
| ``package.json``, but can only package a single application. If the application |
| to ``/app`` within the rock. | ||
| Node.js and npm, inside the rock. Extension discovers location of the | ||
| ``package.json``, but can only package a single application. If application | ||
| defines ``build`` script, development dependencies will be installed, |
Collaborator
There was a problem hiding this comment.
Usage of "a" or "the" depends on if there is only one specific build script or not. Which case is this?
Suggested change
| defines ``build`` script, development dependencies will be installed, | |
| defines a ``build`` script, development dependencies will be installed, |
| Node.js and npm, inside the rock. Extension discovers location of the | ||
| ``package.json``, but can only package a single application. If application | ||
| defines ``build`` script, development dependencies will be installed, | ||
| ``npm run build`` called, and entries matching ``file`` array (excluding |
Collaborator
There was a problem hiding this comment.
Adding verb for consistency with the other two items.
Suggested change
| ``npm run build`` called, and entries matching ``file`` array (excluding | |
| ``npm run build`` will be called, and entries matching ``file`` array (excluding |
| ``package.json``, but can only package a single application. If application | ||
| defines ``build`` script, development dependencies will be installed, | ||
| ``npm run build`` called, and entries matching ``file`` array (excluding | ||
| entries from ``.npmignore``, if exists) will be packages. If ``files`` |
Collaborator
There was a problem hiding this comment.
Suggested change
| entries from ``.npmignore``, if exists) will be packages. If ``files`` | |
| entries from ``.npmignore``, if they exist) will be packages. If the ``files`` |
| 2. The ``package.json`` file should define the ``start`` script. | ||
| For more information, see the `npm documentation <https://docs.npmjs.com/cli/v11/configuring-npm/package-json>`_. | ||
|
|
||
| If application defines ``build`` script in ``package.json`` file, it is |
Collaborator
There was a problem hiding this comment.
For using "a" or "the" in front of build script, see comment above.
Suggested change
| If application defines ``build`` script in ``package.json`` file, it is | |
| If the application defines a ``build`` script in ``package.json`` file, it is |
| For more information, see the `npm documentation <https://docs.npmjs.com/cli/v11/configuring-npm/package-json>`_. | ||
|
|
||
| If application defines ``build`` script in ``package.json`` file, it is | ||
| recommended to have ``files`` array describing the entries to be included |
Collaborator
There was a problem hiding this comment.
Suggested change
| recommended to have ``files`` array describing the entries to be included | |
| recommended to have a ``files`` array describing the entries to be included |
| If application defines ``build`` script in ``package.json`` file, it is | ||
| recommended to have ``files`` array describing the entries to be included | ||
| or have an appropriate ``.npmignore`` file to exclude entries not required | ||
| at the run time. If ``files`` array is not defined and ``.npmignore`` |
Collaborator
There was a problem hiding this comment.
Suggested change
| at the run time. If ``files`` array is not defined and ``.npmignore`` | |
| at the run time. If the ``files`` array is not defined and ``.npmignore`` |
Collaborator
|
Pinging @erinecon for review! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
These changes introduce support for
buildstep that may be defined in package.json file (typescript, vite, etc.)Note: it requires PR#1310 to pass all tests.
make lint && make test.