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

REPL #9365

Merged
merged 40 commits into from Jan 4, 2024
Merged

REPL #9365

merged 40 commits into from Jan 4, 2024

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Nov 7, 2023

@mischnic
Copy link
Member Author

mischnic commented Nov 11, 2023

@devongovett So what you need to do is:

  • Create a Vercel token and set it as REPL_VERCEL_TOKEN
  • Create a new Vercel project (e.g. deploy an empty folder to a new project) for the REPL
    You probably need these settings for that project on vercel.com
Bildschirm­foto 2023-11-11 um 16 24 31
  • From .vercel/project.json take the project and org is and set them as REPL_VERCEL_ORG_ID and REPL_VERCEL_PROJECT_ID

That's used here:

vercel-token: ${{ secrets.REPL_VERCEL_TOKEN }}
vercel-org-id: ${{ secrets.REPL_VERCEL_ORG_ID }}
vercel-project-id: ${{ secrets.REPL_VERCEL_PROJECT_ID }}

For now, this would then deploy as e.g. https://pr-9365.repl.parceljs.org

pr-{{PR_NUMBER}}.repl.parceljs.org

We can add some deploy-on-merge/nightly/release stuff later

@mischnic
Copy link
Member Author

This is what registering the deployment look like in PRs: mischnic#1

@devongovett
Copy link
Member

@mischnic I added the tokens and uploaded an empty project to repl.parceljs.org. I guess we have to merge the GitHub actions changes first in order to test it?

@mischnic
Copy link
Member Author

No, this should work from the PR. It did for mischnic#1

@devongovett
Copy link
Member

Looks like it failed during Vercel deployment:

error An unexpected error occurred: "https://registry.yarnpkg.com/@parcel%2fconfig-repl: Not found".

@mischnic
Copy link
Member Author

mischnic commented Nov 12, 2023

Then you probably haven't disabled building on Vercel itself as I've shown in the screenshot here: #9365 (comment) ?

The switches have to be enabled and the input fields have to be empty. Kind of a weird UI

@devongovett
Copy link
Member

I have this. If I change the override switches but don't fill in a value into the inputs the save button doesn't become enabled...
image

@devongovett
Copy link
Member

changed the build command to "echo build" 😂

@mischnic
Copy link
Member Author

mischnic commented Nov 12, 2023

You're right. But this trick worked for me 🙈

  1. Change "dist" to anything else
  2. Save
  3. Turn on all toggles and make it "dist" again. Then you can save and it does store everything

@devongovett
Copy link
Member

I think it worked! https://pr-9365.repl.parceljs.org

@mischnic
Copy link
Member Author

mischnic commented Nov 12, 2023

You have this enabled for the project (probably globally and you need to disable it for the project): https://vercel.com/docs/security/deployment-protection/methods-to-protect-deployments/vercel-authentication#access-requests

Bildschirm­foto 2023-11-12 um 22 22 27

@mischnic mischnic marked this pull request as ready for review November 12, 2023 21:23
@devongovett
Copy link
Member

ok I turned that off

@mischnic
Copy link
Member Author

mischnic commented Nov 12, 2023

Great, so we're set now.

Apart from the fact that the TypeError: a.FSCache is not a constructor error you get when using the REPL is apparently a regression from #9331 ...

Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty self contained! Will be nice to have this finally merged. 🥳

packages/core/cache/src/LayeredCache.js Outdated Show resolved Hide resolved
@@ -913,7 +913,7 @@ class Directory extends Entry {
}
}

function makeShared(contents: Buffer | string): Buffer {
export function makeShared(contents: Buffer | string): Buffer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these exports use somewhere?

Copy link
Member Author

@mischnic mischnic Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes:

import {MemoryFS, FSError, makeShared, File} from '@parcel/fs';

That implements many of the callback and fd based FS operations so that Yarn works

packages/runtimes/hmr-sse/src/loaders/hmr-runtime.js Outdated Show resolved Hide resolved
const FILENAME = // $FlowFixMe[prop-missing]
process.env.PARCEL_BUILD_REPL && process.browser
? '/app/__virtual__/@parcel/transformer-react-refresh-wrap/src/ReactRefreshWrapTransformer.js'
: __filename;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if we should have a lint rule or something to ensure we don't accidentally add __filename or __dirname again? Also, I guess the default browser replacement that parcel does for those doesn't work in this case?

Copy link
Member Author

@mischnic mischnic Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The problem with the current approach for normal Parcel usage is that runtime assets have a filepath of __filepath and then load require("./loaders/browser/js-loader)from the real filesystem in node_modules. But that filesystem doesn't exist for the REPL, soREPLRuntimesResolver` tries to intercept these dependencies and return the contents of the file. The problem is how to identify these dependencies)

It would be possible to implement this without prefixing all runtime asset paths with /app/__virtual__ (and leave it as is).
But then from the REPLRuntimesResolver perspective (which provides the dependencies of the runtime assets such as the loaders, etc... at runtime), it looks like a dependency with

resolveFrom: '/app/packages/runtimes/js/src/runtime-70a029dfdf59c5ac.js'
specifier: './helpers/bundle-manifest'

and this "internal" dependency is not easily distinguishable from a "user" dependency that should just go through the usual resolver AFAICT. Any ideas?

__filename is packages/runtimes/js/src/JSRuntime.js when building the REPL. Which then gets concatenated onto the project root /app to result in the above resolveFrom

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this now to simply check for these /app/packages/runtimes/js/src/ paths in the resolver.

Comment on lines +7 to +14
// Without this, the hmr-runtime.js is transpiled with the React Refresh swc transform because it
// lives in `/app/packages/runtimes/...` and thus the `config` in the JSTransformer is actually the
// user's package.json, and hmr-runtime.js is transpiled as a JSX asset.
const FILENAME =
// $FlowFixMe
process.env.PARCEL_BUILD_REPL && process.browser
? '/' + __filename
: __filename;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

# Conflicts:
#	yarn.lock
@github-actions github-actions bot temporarily deployed to Preview January 4, 2024 19:07 Inactive
@mischnic mischnic merged commit 61effd4 into v2 Jan 4, 2024
15 of 17 checks passed
@mischnic mischnic deleted the repl-v2-ide branch January 4, 2024 19:25
lettertwo added a commit that referenced this pull request Jan 30, 2024
* upstream/v2: (22 commits)
  Add source map support to the inline-require optimizer (#9511)
  [Web Extension] Add content script world property to manifest schema validation (#9510)
  feat: add getCurrentPackageManager (#9505)
  Default Bundler Contributor Notes (#9488)
  rename parentAsset to root for msb config and remove unstable (#9486)
  Macro errors -> v2 (#9501)
  Statically evaluate constants referenced by macros (#9487)
  Multiple css bundles in Entry bundle groups issue (#9023)
  Fix macro issues (#9485)
  Bump follow-redirects from 1.14.7 to 1.15.4 (#9475)
  Revert more CI changes to centos job (#9472)
  Use lightningcss to implement CSS packager (#8492)
  Fixup CI again (#9471)
  Clippy and use napi's Either3 (#9047)
  Upgrade to eslint 8 (#8580)
  Add support for JS macros (#9299)
  Fixup REPL CI (#9467)
  Drop per-pipeline transformation cache (#9459)
  Upgrade some CI actions (#9466)
  REPL (#9365)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants