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.

Sign up for GitHub

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

Jump to bottom

Add URL prefix to ember app #2238

Merged
merged 5 commits into from
Jan 18, 2024
Merged

Add URL prefix to ember app #2238

merged 5 commits into from
Jan 18, 2024

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jan 17, 2024 •
edited
Loading

Resolves #2235

Do we have a way to test this beforehand?

Locally, it runs at http://localhost:4200/examples/ember/

@NullVoxPopuli NullVoxPopuli mentioned this pull request Jan 17, 2024
Closed
@flashdesignory
Copy link
Collaborator

@NullVoxPopuli - you can test it by spinning up the whole website locally.
Please ensure to use the correct node version, otherwise it won't install correctly.
Once done you should be able to run npm run gulp serve and then go to localhost:8080

Ideally you'd update the link in the index page and learn.json file as well to point to the dist folder.

flashdesignory
flashdesignory reviewed Jan 18, 2024
View reviewed changes
examples/emberjs/todomvc/config/environment.js Outdated
// rootURL: '',
// locationType: 'none',
rootURL: '/',
rootURL: '/examples/ember/dist/',
Copy link
Collaborator

@flashdesignory flashdesignory Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

should this point to the 'emberjs' folder instead of 'ember' ?
similar the other references in the index.html file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably -- that's why I asked for a local way to run the server or build the repo, so I could verify 💪

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should probably also be /examples/emberjs/todomvc/dist if the structure matches what's in the repo?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would probably only change the rootURL for production, since you want to be able to work locally in the emberjs/todomvc directory and spin up the app directly.

How about this:

if (environment === 'production') {
		// here you can enable a production-specific feature
    ENV.rootURL = '/examples/emberjs/todomvc/dist/';
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the URL prefix is still usable in development locally

@NullVoxPopuli
Copy link
Contributor Author

Please ensure to use the correct node version, otherwise it won't install correctly.

woah, Node 8.17.0.

installation takes forever 🙃

Once done you should be able to run npm run gulp serve and then go to localhost:8080

thanks!

Ideally you'd update the link in the index page and learn.json file as well to point to the dist folder.

can do!

@NullVoxPopuli
Copy link
Contributor Author

hm, installation isn't succeeding for me, I'm getting this:

❯ NODE_OPTIONS="" npm i
^C           ......] \ extract:unicode-5.2.0: sill extract [email protected] extracted to /home/nvp/Development/OpenSource/todomvc/node_modules/.stagi
npm ERR! cb() never called!

npm ERR! This is an error with npm itself. Please report this error at:
npm ERR!     <https://npm.community>

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/nvp/.npm/_logs/2024-01-18T17_04_56_168Z-debug.log

(I've had to ctrl+c, because it hung on extracting unicode)

@flashdesignory
Copy link
Collaborator

hmm I don't remember running into that issue.
I can run your branch locally though, so if it's too much trouble I can verify for you.

flashdesignory
flashdesignory reviewed Jan 18, 2024
View reviewed changes
examples/emberjs/todomvc/dist/index.html Outdated




<link integrity="" rel="stylesheet" href="/assets/todomvc.d41d8cd98f00b204e9800998ecf8427e.css">
<link integrity="" rel="stylesheet" href="/examples/ember/dist/assets/todomvc.d41d8cd98f00b204e9800998ecf8427e.css">
Copy link
Collaborator

Choose a reason for hiding this comment

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

with the above change and running build:production, the new output should automatically have the correct path appended.

@flashdesignory
Copy link
Collaborator

flashdesignory commented Jan 18, 2024 •
edited
Loading

can we also change the links in the index.html and learn.json?

<a class="applist-item" href="examples/emberjs/todomvc/dist/" ... </a>

@NullVoxPopuli
Copy link
Contributor Author

hmm I don't remember running into that issue.

any chance node can be upgraded to a supported version? 😅

@NullVoxPopuli
Copy link
Contributor Author

can we also change the links in the index.html and learn.json?

I've guessed at these values in the latest commit(s).

@flashdesignory
Copy link
Collaborator

any chance node can be upgraded to a supported version? 😅

It's on our todo list 😄

flashdesignory
flashdesignory approved these changes Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@flashdesignory flashdesignory left a comment

Choose a reason for hiding this comment

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

LGTM 😄
nit: we could delete emberjs/assets and emberjs/index.html I believe?

@NullVoxPopuli
Copy link
Contributor Author

nit: we could delete emberjs/assets and emberjs/index.html I believe?

oh yeah, those symlinks -- those confused me, but now I understand why they were there. They are deleted.

@flashdesignory flashdesignory merged commit efafb58 into tastejs:master Jan 18, 2024
@NullVoxPopuli NullVoxPopuli deleted the prefix-url-ember branch January 18, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flashdesignory flashdesignory flashdesignory approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ember example link broken
2 participants
@NullVoxPopuli @flashdesignory