Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Whilst contributors are responsible for transpiling .es6.js to .js it'll be good to have a command to check if it needs doing. This will help us build automated tools that can fail patches prior to commit so we don't make mistakes.
Proposed resolution
Add a check:js
command to package.json that unlike build or watch does not write the files but just fails if the files are not up-to-date.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#13 | 2880013-2-13.patch | 6.03 KB | alexpott |
#13 | 9-13-interdiff.txt | 883 bytes | alexpott |
#9 | c20170521_184113.png | 20.6 KB | droplet |
#9 | interdiff.patch | 1.61 KB | droplet |
#9 | 2880013-8.patch | 5.95 KB | droplet |
Comments
Comment #2
alexpottI think we might be able to refactor the tooling there is less duplication between this and the other commands... maybe a js expert can help :)
Comment #3
droplet CreditAttribution: droplet commenteddoesn't it:
then meant:
Comment #4
alexpott@droplet the point is to have a command so that scripts can check without actually changing the code.
Comment #5
alexpottLet's do the transpiling the same piece of code to make it easier to maintain.
Comment #6
droplet CreditAttribution: droplet commentedHmm.. Or this way:
No Yarn.lock updates, run
yarn why minimist
to get more info :)Comment #7
alexpottI still think it is work separating the writing and checking concerns and also only doing the argument parsing in one place. Helps mitigate side-effects. Patch attached keeps the
--check
flag but moves all the argument handling to babel-es-build.js.Comment #9
droplet CreditAttribution: droplet commented@alexpott that works.
Also, I think we want to collect all useful info rather than throw error to stop script immediately
(highlighted box is after patched)
Comment #10
alexpott@droplet thanks for making that change!
This is looking really good now... I've confirmed that the process exits with an error when there is one. Like it.
Comment #12
GrandmaGlassesRopeManShould say
yarn run build:js
Just a minor nit about the docs. Other than that I like this and the separation of the code into smaller, easier to test parts, makes me happy.
Comment #13
alexpottUpdated the patch for #12
Comment #14
GrandmaGlassesRopeMan@alexpott
Great. I like this and it's probably important to get this in before we start working on actual improvements to core JavaScript.
Comment #15
catchCommitted/pushed to 8.4.x, thanks!
Comment #17
alexpottThis just helped me find #2880560: Fix comment added to top of .js files by build process